-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Populate ActionDescriptor EndpointMetadata #34065
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
Populate ActionDescriptor EndpointMetadata #34065
Conversation
I used the below change to app.MapGet("/json", [ApiExplorerSettings(IgnoreApi = true)] (IApiDescriptionGroupCollectionProvider provider) =>
{
if (provider.ApiDescriptionGroups is not null)
{
// Used to inspect and debug ApiDescriptionGroups
}
return Json(new Person("John", 42));
}); |
Also, with this and #34063, it's possible there's other things missing from the |
Populate the EndpointMetadata property of the ActionDescriptor on ActionDescriptor instances for minimal actions so that the attributes associated with the request delegate can be inspected. Relates to dotnet#34061.
74bf7b5
to
2032c68
Compare
Digging into it more this isn't a magic fix for #34061 for For example: builder.Services.AddSwaggerGen(options =>
{
options.SwaggerDoc("v1", new OpenApiInfo { Title = "Todo API", Version = "v1" });
options.DocInclusionPredicate(
(_, description) =>
!description.ActionDescriptor.EndpointMetadata.OfType<ApiExplorerSettingsAttribute>().Any((p) => p.IgnoreApi));
}); |
It's not ok for API explorer to modify the endpoint metadata on action descriptor. The action descriptors should only be modified by the IActionDescriptor providers. While this potentially solves the problem you are facing, we don't think this is the correct way to solve it. The endpoint metadata on the action descriptor is "Incomplete" by design and the one on the actual endpoint should be used instead. The metadata on the descriptor only reflects the metadata at the time the descriptor was created and doesn't include anything that has been added after the endpoint was generated. Updating it there, means that other libraries will see a different result after navigating to the swagger page and that is not acceptable. |
Then is it the case that part of the implementation in the provider is in the wrong place, given that the aspnetcore/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs Lines 92 to 99 in 2032c68
The class modified here implements It's also not really modified per se, as it's just completely empty - there is no metadata at all, as it was constructed just a few lines earlier. The application has no prior access to the If that's still incorrect, then I'll close this PR. In that case though, what is the intended design that would allow the action's metadata to be accessible to the API Explorer for inspection? |
@martincostello I might have overlooked something, it's been a while since I've been in that part of the codebase. I'll take another look and update my comment here as necessary. |
// Intentionally not readonly to prevent defensive struct copies | ||
private readonly object[] _items; | ||
private object[] _items; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
Remove filtering and just directly populate the list. Fix typo in comment.
I think we should expose all the method's attributes on the ActionDescriptors generated by EndpointMetadataApiDescriptionProvider somehow, and those are all in RouteEndpoint.Metadata. This makes a lot of sense to me if @pranavkm has no objections. |
I was literally just looking for this trying to configure a custom operation id builder for Swashbuckle for minimal APIs. This will let us get to the endpoint name. |
Hi @DamianEdwards. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
PR Title
Populate
ActionDescriptor.EndpointMetadata
.PR Description
Populate the
EndpointMetadata
property of theActionDescriptor
onApiDescription
instances for minimal actions so that the attributes associated with the request delegate can be inspected.This seems to me like it's something that should be done and is just missing, but in case it was intentional and this use case is intended to be solved in a different way, I've just opened this as a draft for now pending confirmation (or not) of the principle.
Relates to #34061.