OpenAPI: Apply descriptions from [Description] to the schema reference instead of the actual schema for properties #63177
Conversation
github-actions
bot
added
the
needs-area-label
dotnet-policy-service
bot
added
the
community-contribution
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where property descriptions from [Description] attributes were incorrectly applied to the actual schema instead of the schema reference when dealing with complex types in OpenAPI schema generation.
- Updates the schema service to differentiate between inlined and referenced schemas when applying descriptions
- Adds support for x-reference metadata annotations in the JSON schema
- Introduces comprehensive test coverage for both referenced and inlined schema scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
OpenApiSchemaService.cs |
Modified schema generation logic to apply property descriptions to reference annotations for non-inlined schemas while preserving existing behavior for inlined schemas |
OpenApiJsonSchema.Helpers.cs |
Added support for reading reference description and example annotations from JSON schema metadata |
OpenApiSchemaService.Annotations.cs |
Added comprehensive test cases covering both referenced and inlined schema scenarios with various description attribute combinations |
gfoidl
added
area-mvc
| if (attributeProvider.GetCustomAttributes(inherit: false).OfType<DefaultValueAttribute>().LastOrDefault() is DefaultValueAttribute defaultValueAttribute) | ||
| var propertyAttributes = attributeProvider.GetCustomAttributes(inherit: false); | ||
| var isInlinedSchema = schema[OpenApiConstants.SchemaId] is null; | ||
| if (isInlinedSchema) |
There was a problem hiding this comment.
I keep squinting at this and trying to figure out if it is a break that we now only apply validation attributes and default value attributes to property types that are inlined. I think we're fine since those attributes will typically only be applied to primitive values but this does feel a little spooky. Maybe we can reserve the isInlinedSchema just for the description stuff?
There was a problem hiding this comment.
I can do that, but I think it's more correct to not apply them for things which becoma a schema reference. It can only work when the schema of that property becomes a merged object using AllOf tricks. But even then, there are no Json Schema validations around object (except for a sub schema which is referenced in this case).
One semi-exception (before 3.1) was the required attribute, but that is already handled in the TypeInfoResolverModifier.
For validations and modifications on schemas which will be referenced it becomes a problem when two properties for the same schema give conflicting information, because they would both apply the attributes, which makes the last property overwrite the validations of the first property.
Let me whether to keep it like this or make it limited to the description 😊. I can also add a comment explaining this in the code.
There was a problem hiding this comment.
I updated it to be limited to the description only, because I am going on holiday for 2 weeks. Then I will have time to add more tests for the validation attributes, and default value. So we can be confident to exclude them for schema references.
|
@captainsafia thank you for your review. I've removed the x-ref-example handling in the JsonSchema exporter as there is no [Example] attributes. I've replied to the second comment let me know if you agree, or that I limit it to the description. |
|
On the comment for the SchemaService, I've now limited the change to only the |
Thanks! I generally agree that not applying validation/default valeus to references is probably the correct thing to do but the change felt subtle enough that we should sleep on it. Enjoy your holiday and I look forward to more PRs when you get back! 😄 |
3 participants
OpenAPI: Apply descriptions from [Description] to the schema reference instead of the actual schema for properties
Update the json schema exporter for openapi to apply x-reference metadata for references.
Description
When a property of a class is a complex type and becomes a reference, the description was wrongfully applied to the actual schema instead of the reference.
Related previous PR #62213
Fixes #63175