Skip to content

Fix up handling for awaitable IResults in ApiExplorer #57068

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 1 commit into from
Jul 31, 2024
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
7 changes: 7 additions & 0 deletions src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,13 @@ internal static Dictionary<int, ApiResponseType> ReadResponseMetadata(

foreach (var metadata in responseMetadata)
{
// `IResult` metadata inserted for awaitable types should
Copy link
Member

Choose a reason for hiding this comment

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

I can't tell from this comment whether IResult only appears because we inserted it. Is there a case where it's present legitimately and we shouldn't skip it?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's theoritically feasible that IResult could appear there if someone manually insert it by doing something like:

app.MapGet()
	.Produces<IResult>("application/json");

In either case, it still stands that IResult doesn't provide us with type information specific enough to include in the response type information.

Copy link
Member

Choose a reason for hiding this comment

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

What about something like "IResult (usually inserted for awaitable types) isn't useful..."?

// not be considered for response metadata.
if (typeof(IResult).IsAssignableFrom(metadata.Type))
{
continue;
}

var statusCode = metadata.StatusCode;

var apiResponseType = new ApiResponseType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,12 +324,6 @@ private static void AddSupportedResponseTypes(
{
var responseType = returnType;

// Can't determine anything about IResults yet that's not from extra metadata. IResult<T> could help here.
if (typeof(IResult).IsAssignableFrom(responseType))
{
responseType = typeof(void);
}

// We support attributes (which implement the IApiResponseMetadataProvider) interface
// and types added via the extension methods (which implement IProducesResponseTypeMetadata).
var responseProviderMetadata = endpointMetadata.GetOrderedMetadata<IApiResponseMetadataProvider>();
Expand All @@ -338,6 +332,14 @@ private static void AddSupportedResponseTypes(
var defaultErrorType = errorMetadata?.Type ?? typeof(void);
var contentTypes = new MediaTypeCollection();

// If the return type is an IResult or an awaitable IResult, then we should treat it as a void return type
// since we can't infer anything without additional metadata.
if (typeof(IResult).IsAssignableFrom(responseType) ||
producesResponseMetadata.Any(metadata => typeof(IResult).IsAssignableFrom(metadata.Type)))
{
responseType = typeof(void);
}

var responseProviderMetadataTypes = ApiResponseTypeProvider.ReadResponseMetadata(
responseProviderMetadata, responseType, defaultErrorType, contentTypes, out var errorSetByDefault);
var producesResponseMetadataTypes = ApiResponseTypeProvider.ReadResponseMetadata(producesResponseMetadata, responseType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Security.Claims;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Infrastructure;
Expand Down Expand Up @@ -273,6 +274,144 @@ public void AddsMultipleResponseFormatsFromMetadataWithIResult()
Assert.Empty(badRequestResponseType.ApiResponseFormats);
}

[Fact]
public void AddsMultipleResponseFormatsForTypedResults()
{
var apiDescription = GetApiDescription(Results<Created<InferredJsonClass>, BadRequest> () =>
TypedResults.Created("https://example.com", new InferredJsonClass()));

Assert.Equal(2, apiDescription.SupportedResponseTypes.Count);

var createdResponseType = apiDescription.SupportedResponseTypes[0];

Assert.Equal(201, createdResponseType.StatusCode);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.Type);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.ModelMetadata?.ModelType);

var createdResponseFormat = Assert.Single(createdResponseType.ApiResponseFormats);
Assert.Equal("application/json", createdResponseFormat.MediaType);

var badRequestResponseType = apiDescription.SupportedResponseTypes[1];

Assert.Equal(400, badRequestResponseType.StatusCode);
Assert.Equal(typeof(void), badRequestResponseType.Type);
Assert.Equal(typeof(void), badRequestResponseType.ModelMetadata?.ModelType);

Assert.Empty(badRequestResponseType.ApiResponseFormats);
}

[Fact]
public void AddsResponseFormatsForTypedResultWithoutReturnType()
{
var apiDescription = GetApiDescription(() => TypedResults.Created("https://example.com", new InferredJsonClass()));

Assert.Equal(1, apiDescription.SupportedResponseTypes.Count);

var createdResponseType = apiDescription.SupportedResponseTypes[0];

Assert.Equal(201, createdResponseType.StatusCode);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.Type);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.ModelMetadata?.ModelType);

var createdResponseFormat = Assert.Single(createdResponseType.ApiResponseFormats);
Assert.Equal("application/json", createdResponseFormat.MediaType);
}

[Fact]
public void HandlesTypedResultsWithoutIEndpointMetadataProviderImplementation()
{
// TypedResults for ProblemDetails doesn't implement IEndpointMetadataProvider
var apiDescription = GetApiDescription(() => TypedResults.Problem());

Assert.Equal(1, apiDescription.SupportedResponseTypes.Count);

var responseType = apiDescription.SupportedResponseTypes[0];

Assert.Equal(200, responseType.StatusCode);
Assert.Equal(typeof(void), responseType.Type);
Assert.Equal(typeof(void), responseType.ModelMetadata?.ModelType);
}

[Fact]
public void AddsResponseFormatsForAwaitableTypedResultWithoutReturnType()
{
var apiDescription = GetApiDescription(() =>
Task.FromResult(TypedResults.Created("https://example.com", new InferredJsonClass())));

Assert.Equal(1, apiDescription.SupportedResponseTypes.Count);

var createdResponseType = apiDescription.SupportedResponseTypes[0];

Assert.Equal(201, createdResponseType.StatusCode);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.Type);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.ModelMetadata?.ModelType);

var createdResponseFormat = Assert.Single(createdResponseType.ApiResponseFormats);
Assert.Equal("application/json", createdResponseFormat.MediaType);
}

// Coverage for https://github.com/dotnet/aspnetcore/issues/56975
[Fact]
public void AddsMultipleResponseFormatsFromMetadataWithAwaitableIResult()
{
var apiDescription = GetApiDescription(
[ProducesResponseType(typeof(InferredJsonClass), StatusCodes.Status201Created)]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
() => Task.FromResult(Results.Ok(new InferredJsonClass())));

Assert.Equal(2, apiDescription.SupportedResponseTypes.Count);

var createdResponseType = apiDescription.SupportedResponseTypes[0];

Assert.Equal(201, createdResponseType.StatusCode);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.Type);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.ModelMetadata?.ModelType);

var createdResponseFormat = Assert.Single(createdResponseType.ApiResponseFormats);
Assert.Equal("application/json", createdResponseFormat.MediaType);

var badRequestResponseType = apiDescription.SupportedResponseTypes[1];

Assert.Equal(400, badRequestResponseType.StatusCode);
Assert.Equal(typeof(void), badRequestResponseType.Type);
Assert.Equal(typeof(void), badRequestResponseType.ModelMetadata?.ModelType);

Assert.Empty(badRequestResponseType.ApiResponseFormats);
}

[Fact]
public void AddsMultipleResponseFormatsFromMetadataWithAwaitableResultType()
{
var apiDescription = GetApiDescription(
[ProducesResponseType(typeof(InferredJsonClass), StatusCodes.Status201Created)]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
async Task<Results<Created<InferredJsonClass>, ProblemHttpResult>> () => {
await Task.CompletedTask;
return Random.Shared.Next() % 2 == 0
? TypedResults.Created<InferredJsonClass>("/", new InferredJsonClass())
: TypedResults.Problem();
});

Assert.Equal(2, apiDescription.SupportedResponseTypes.Count);

var createdResponseType = apiDescription.SupportedResponseTypes[0];

Assert.Equal(201, createdResponseType.StatusCode);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.Type);
Assert.Equal(typeof(InferredJsonClass), createdResponseType.ModelMetadata?.ModelType);

var createdResponseFormat = Assert.Single(createdResponseType.ApiResponseFormats);
Assert.Equal("application/json", createdResponseFormat.MediaType);

var badRequestResponseType = apiDescription.SupportedResponseTypes[1];

Assert.Equal(400, badRequestResponseType.StatusCode);
Assert.Equal(typeof(void), badRequestResponseType.Type);
Assert.Equal(typeof(void), badRequestResponseType.ModelMetadata?.ModelType);

Assert.Empty(badRequestResponseType.ApiResponseFormats);
}

[Fact]
public void AddsFromRouteParameterAsPath()
{
Expand Down
Loading