`RequestDelegate`-based handlers do not surface in OpenAPI descriptions
YataoFeng opened this issue · 13 comments
Is there an existing issue for this?
- I have searched the existing issues
Describe the bug
hi I found that the minimal Api TypedResults method does not automatically inject HttpContext.
The httpcontext principle is here in the reference
https://learn.microsoft.com/en-us/aspnet/core/fundamentals/minimal-apis?view=aspnetcore-7.0#binding-precedence
Expected Behavior
The TypedResults method can automatically inject HttpContext.
Steps To Reproduce
File:Program.cs
var builder = WebApplication.CreateBuilder(args);
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();
var app = builder.Build();
if (app.Environment.IsDevelopment())
{
app.UseSwagger();
app.UseSwaggerUI();
}
app.MapGroup("/connect").MapConnect();
app.Run();
File:Connect.cs
public static class Connect
{
public static RouteGroupBuilder MapConnect(this RouteGroupBuilder group)
{
group.MapGet("/authorize", AuthorizeAsync);
group.MapGet("/GoodMinimalApi", GoodMinimalApi);
return group;
}
public static async Task<IResult> GoodMinimalApi() // not HttpContext
{
await Task.CompletedTask;
return TypedResults.Ok();
}
public static async Task<IResult> AuthorizeAsync(HttpContext context)
{
await Task.CompletedTask;
return TypedResults.Ok();
}
}
End result
Exceptions (if any)
No response
.NET Version
dotnet 7.0
Anything else?
Microsoft Visual Studio Enterprise 2022 (64 位) - Current版本 17.4.0
ASP.NET Core 7
Minimal Api
This is a problem with Swashbuckle.AspNetCore
This is a problem with Swashbuckle.AspNetCore
3q
Does updating
group.MapGet("/authorize", AuthorizeAsync);
to
group.MapGet("/authorize", (Delegate)AuthorizeAsync);
fix your issue?
It looks like you're getting the RequestDelegate
overload of MapGet when you need the Delegate
overload in order to process the Task<T>
(specifically Task<IResult>
) overload. @JamesNK added an analyzer to warn about this and recommend this workaround described #44316.
This didn't get added in time for the .NET 7 release though. Is this something we could backport to the .NET 7 SDK in a servicing release? I think there could be false positives, so that might be risky.
We wanted to just make this work (see see #40235), but this caused issues for trimming and AOT. Using source generators, we might be able to just make this work once a gain but in a way that's trim friendly.
Also, if you want to get the best swagger output for TypedResults, you want to include the specific TypedResult in the return type. Task<IResult>
loses information. Task<Ok>
indicates that the route handler will return a 200 response. This might be the default assumption, but this becomes more important with different status codes (e.g. Task<NotFound>
or Task<Results<Ok, NotFound>>
) or with a specific schema (e.g. Task<Ok<MyDto>>
).
So you should prefer
public static async Task<Ok> GoodMinimalApi()
over
public static async Task<IResult> GoodMinimalApi()
You can read more about TypedResults
and Results<TResult1, ..., TResultN>
at https://learn.microsoft.com/en-us/aspnet/core/fundamentals/minimal-apis/responses?view=aspnetcore-7.0#resultstresult1-tresultn.
This is a problem with Swashbuckle.AspNetCore
This issue actually starts in ASP.NET Core. Our OpenAPI infrastructure relies on MethodInfo
being available in EndpointMetadata
. However, as mentioned above, since your code is resolving to the RequestDelegate
overload of MapGet
, we explicitly don't add method info to the metadata in this case.
In .NET 7 RC1, we actually enabled adding MethodInfo
to metadata for all types of endpoints (ones using RequestDelegate
or Delegate
) but we got feedback that this was resulting in undesired behavior (see #44005) so we undid it.
On solution we discussed was making it so that calling WithOpenApi
on an endpoint would add it to OpenAPI definition regardless of the delegate type. This would mean your problem would be solved by calling group.MapGet("/authorize", AuthorizeAsync).WithOpenApi();
or app.MapGroup("/connect").MapConnect().WithOpenApi();
.
Does that seem like a sensible solution?
cc: @martincostello for thoughts as well
I like the idea of making it opt-in via something like WithOpenApi()
.
My main concern (and how I found #44005) was that third party code might start exposing unintended endpoints that are difficult to suppress from the OpenAPI document (if you notice them even appear) if you can't update the source of the endpoint to mark it has hidden.
I like the idea of making it opt-in via something like
WithOpenApi()
.My main concern (and how I found #44005) was that third party code might start exposing unintended endpoints that are difficult to suppress from the OpenAPI document (if you notice them even appear) if you can't update the source of the endpoint to mark it has hidden.
Thanks for the feedback! Yeah, I'm thinking that by leveraging WithOpenApi
as an opt-in and introducing support for #44192, there will be flexibility to include and omit these RequestDelegate
-based endpoints as needed.
Thanks for contacting us.
We're moving this issue to the .NET 8 Planning
milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.
I started working on a PR for this (#47346) and ran into a little snafu when defining the OpenAPI operation, specifically around how the HttpContext
parameter is handled.
Ultimately, I decided that by default, an HttpContext
parameter would not be emitted to the OpenAPI spec since HttpContext
isn't a deserializable or parsable parameter type and doesn't represent a public API for the endpoint. Users can always modify this by configuring their own parameters in the WithOpenApi
extension method.
This means that when testing with the Swagger UI, you'll only be able to send parameter-less and body-less requests to the endpoint for testing. This restriction makes sense to me. Thoughts?
Ignoring HttpContext
parameters when building the OpenAPI operation makes sense to me too. You can't infer anything about what the route handler is doing just from knowing it takes in an HttpContext
. Is there really any alternative?
I am using the Delegate overload of MapGet but my issue is slightly different:
I have a response dto being returned by the delegate whose display title I would like to customize in Swagger. While using controllers, decorating the response model with a SwaggerSchemaAttribute
would do the trick like so:
[SwaggerSchema(Title = "ResponseNameToShowInSwagger", Description = "I expect to see this description is swagger.")]
public class MyResponse
{
}
However, this no longer seems to be respected by minimal APIs. I just want to know whether the issue being tracked here will solve my problem as well or could there a different cause of my issue? Will appreciate any heads up on this.
@syedsuhaib Can you file a separate issue for this? I can provide guidance there...
Workaround:
private static IEndpointConventionBuilder MapGetWithOpenAPI(this IEndpointRouteBuilder builder, [StringSyntax("Route")] string path, RequestDelegate requestDelegate)
{
return builder.MapGet(path, requestDelegate).WithMetadata(requestDelegate.Method);
}
See: #44005 (comment)