Fix mapping for nested schemas and [Produces] attributes in OpenAPI implementation #57852
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
captainsafia
added
feature-openapi
area-minimal
captainsafia
added
the
area-mvc
BrennanConroy
approved these changes
Sep 13, 2024
| // reference by duplicate rules. | ||
| if (schema == null || currentDepth > MaxSchemaReferenceRecursionDepth) | ||
| { | ||
| return; |
There was a problem hiding this comment.
Yep, we should be able to inject a logger into this codepath.
There was a problem hiding this comment.
Hmmm...actually I think we should throw here...
captainsafia
added
the
Servicing-consider
halter73
reviewed
Sep 13, 2024
| @@ -14,6 +14,8 @@ namespace Microsoft.AspNetCore.OpenApi; | |||
| /// </summary> | |||
| internal sealed class OpenApiSchemaStore | |||
| { | |||
| // Matches default MaxValidationDepth and MaxModelBindingRecursionDepth used by MVC. | |||
| private readonly int MaxSchemaReferenceRecursionDepth = 32; | |||
There was a problem hiding this comment.
MaxValidationDepth and MaxModelBindingRecursionDepth are both configurable via MvcOptions. Do we think this will need to be configurable too?
There was a problem hiding this comment.
In a future release, yes, but we will have to live with it being un-configurable for .NET 9 for now. :/
There was a problem hiding this comment.
Per off-line discussion, we're relying on STJ's recursion depth check which do have a configurable option via JsonSerializerOptions.MaxDepth.
This was referenced Sep 14, 2024
captainsafia
added
Servicing-approved
|
Approved via email. |
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR consists of two changes to address feedback received after .NET 9 RC 1 for the new APIs in the Microsoft.AspNetCore.OpenApi package.
[Produces]attribute with no-type would override the schema for a pre-existing schema by replacing direct assignment for content types withTryAddinOpenApiDocumentService.GetResponseAsync.OpenApiSchemaStore.PopulateSchemaIntoReferenceCache.Fixes #57799
Customer Impact
This PR resolves issues reported in the OpenAPI implementation that don't currently have any viable workarounds for users. Without these changes, documents generated by this implementation may unintentionally drop schemas for responses generated controller actions with certain attributes. This change also stabilizes the generations of schema names for deeply nested type hierarchies in the OpenAPI document.
Regression?
Risk
Changes in this PR only impact the
Microsoft.AspNetCore.OpenApiand don't affect any APIs in the shared framework. Changes are in new code paths that were introduced in .NET 9 so there is no risk of version-to-version regression.Verification
Packaging changes reviewed?