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

Update responses section with controllers metadata information #33739

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

mikekistler
Copy link
Contributor

@mikekistler mikekistler commented Sep 27, 2024

Update responses section to describe how to add OpenAPI response metadata for controller-based apps.


Internal previews

📄 File 🔗 Preview link
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Generate OpenAPI documents

@mikekistler
Copy link
Contributor Author

@Rick-Anderson @tdykstra @captainsafia I think this is ready for review / feedback.

Copy link
Contributor

@Rick-Anderson Rick-Anderson left a comment

Choose a reason for hiding this comment

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

Check my changes, I might have changed the meaning in some.

I'll do a PR after this merges to wordsmith from ##### [Controllers](#tab/controllers) on.

aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
mikekistler and others added 2 commits October 1, 2024 05:22
Co-authored-by: Rick Anderson <3605364+Rick-Anderson@users.noreply.github.com>
Copy link
Contributor

@tdykstra tdykstra left a comment

Choose a reason for hiding this comment

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

Looks good; just a few minor issues.

aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
aspnetcore/fundamentals/openapi/aspnetcore-openapi.md Outdated Show resolved Hide resolved
Co-authored-by: Tom Dykstra <tdykstra@microsoft.com>
* the schema for the response body of 3xx and 5xx responses is considered to be not specified,
* the content-type for the response body can be inferred from the return type of the action method and the set of output formatters.

Note that there is no build-time validation of the OpenAPI metadata for responses in controller-based apps. For example, there is no build error issued if an action method returns a different status code than one specified by a <xref:Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute> attribute, and may return an object of a different type than specified in the <xref:Microsoft.AspNetCore.Mvc.ActionResult%601> return type of the action method.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@captainsafia Please give this paragraph extra scrutiny.

Copy link
Member

Choose a reason for hiding this comment

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

Ditto the above comment about build-time vs run-time.

One caveat with MVC is that we do have analyzers that might actually catch some inconsistencies although I dunno if we want to wade into the weeds with that...


OpenAPI supports providing a description of the responses returned from an API. Minimal APIs support three strategies for setting the response type of an endpoint:
Similar to controller-based apps, there is no build-time validation of the OpenAPI metadata specified with the <xref:Microsoft.AspNetCore.Http.OpenApiRouteHandlerBuilderExtensions.Produces%2A> extension method or the <xref:Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute> attribute. For example, there is no build error issued:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@captainsafia Another part to give extra scrutiny.

Copy link
Member

@captainsafia captainsafia Oct 3, 2024

Choose a reason for hiding this comment

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

"build-time validation" feels like an unfamiliar turn of phrase for me, but I'm having trouble coming up with better alternatives. Perhaps this is fine given that you provide examples below.

Update: also I think what we mean here is "run-time validation"? As in these attributes don't directly change behavior of the API?

* Via the [`ProducesResponseType`](xref:Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute) attribute on the route handler.
* By returning<xref:Microsoft.AspNetCore.Http.TypedResults> from the route handler.
* If an endpoint returns a different status code than specified by a <xref:Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute> attribute.
* When the endpoint method returns an object of a different type than specified in the <xref:Microsoft.AspNetCore.Http.OpenApiRouteHandlerBuilderExtensions.Produces%2A> extension method.
Copy link
Member

Choose a reason for hiding this comment

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

Should we add that the content-types don't influence content negotiation either?

Co-authored-by: Safia Abdalla <safia@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants