-
Notifications
You must be signed in to change notification settings - Fork 10.5k
fix: Ensure OpenAPI request bodies include (#59042) #64859
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
base: main
Are you sure you want to change the base?
fix: Ensure OpenAPI request bodies include (#59042) #64859
Conversation
`DescriptionAttribute` for `[FromForm]` parameters and add verification tests.
|
@dotnet-policy-service agree |
desjoerd
left a comment
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.
Thanks for your contribution.
I've added a few comments, could you take a look at them? 😊
| { | ||
| parameterSchema = new OpenApiSchema | ||
| { | ||
| Description = parameterDescription, |
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.
If this is added because it will be componetized (and a $ref) you can set the description on the final $ref via the metadata x-ref-description. Or is there another reason? As this would change the output to emit an allOf.
| { | ||
| propSchema = new OpenApiSchema | ||
| { | ||
| Description = parameterDescription, |
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.
idem
| { | ||
| propSchema = new OpenApiSchema | ||
| { | ||
| Description = parameterDescription, |
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.
idem
| using Microsoft.AspNetCore.OpenApi; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
|
|
||
| public class ReproduceIssue59042Tests : OpenApiDocumentServiceTestBase |
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.
I think these tests would best fit in OpenApiDocumentServiceTests.Parameters.cs
|
This change was not added specifically for componentization. The goal is to preserve the description in the generated schema output without relying on ref metadata. I also followed your suggestion: for componentized schemas emitted as $ref, the description is now stored in Metadata using x-ref-description. This avoids introducing an unnecessary allOf while preserving the description. Additionally, I moved the related unit test to OpenApiDocumentServiceTests.Parameters.cs as you recommended. The implementation should now behave as intended, but I’m happy to adjust further if needed. |
|
Looks good to me! @captainsafia could you review this PR as well? :) |
DescriptionAttributefor[FromForm]parameters and add verification tests.fix: Ensure OpenAPI request bodies include (#59042)
Fixes caching of OpenApiOperationTransformerContext to prevent exceptions in OpenAPI document generation.
Description
Corrected caching logic for OpenApiOperationTransformerContext in OpenApiDocumentService.
Ensured transformer contexts are properly initialized and stored in _operationTransformerContextCache.
Added handling for operation metadata to avoid cache misses.
Verified with all existing unit and integration tests for OpenAPI document generation.
Fixes #59042