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

Add default implementation to OpenAPI's new Description property to prevent a breaking change #60937

Merged
merged 3 commits into from
Mar 17, 2025

Conversation

sander1095
Copy link
Contributor

@sander1095 sander1095 commented Mar 14, 2025

Add default implementation to OpenAPI's new Description property to prevent a breaking change

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

This PR adds a default implementation to IProducesResponseTypeMetadata.Description and IApiResponseMetadataProvider.Description to prevent breaking changes.

Description

In #58193 I added a Description property to IProducesResponseType, IApiResponseMetadataProvider, attributes and some other spots to allow developers to easily set response descriptions.

The linked issue tries to cast their own class implementation to IProducesResponseType. Because they don't have Description, an exception is thrown.

This issue is fixed by adding a default implementation of this property.

Fixes #59804

…e when consumers cast this type and get exceptions about description being null
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Mar 14, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 14, 2025
@sander1095
Copy link
Contributor Author

@captainsafia I just created this quick PR with the desired change you mentioned in the issue.

I want to double check something. The original PR (#58193) adds the property description to multiple places, both classes (and attributes) and an interface (src/Mvc/Mvc.Core/src/ApiExplorer/IApiResponseMetadataProvider.cs).

Would you like me to add a default implementation in more places to avoid this issue to from showing up again in the future? If so, can you provide some direction for which classes this is desired?

If desired, I could also proceed by adding some unit tests that cast the interface the same way the issue does to prevent regression, but I am not sure if this is too specific as we simply missed this in API review.

Co-authored-by: Safia Abdalla <safia@safia.rocks>
@sander1095
Copy link
Contributor Author

sander1095 commented Mar 14, 2025

From a chat with Safia, a TODO for me:

As far as testing for this, maybe a quick sanity check to see if an implementation of the interface without the property will work fine

Just to be safe we should do a default implementation on all the interfaces that implement it.

@sander1095 sander1095 requested a review from a team as a code owner March 16, 2025 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
2 participants