- 
                Notifications
    You must be signed in to change notification settings 
- Fork 10.5k
OpenAPI: Apply descriptions from [Description] to the schema reference instead of the actual schema for properties #63177
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
Changes from all commits
e7047ac
              6cd0d41
              3bafc2a
              91e890a
              d78e47e
              7ff6c0d
              4af724c
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
|  | @@ -101,19 +101,35 @@ internal sealed class OpenApiSchemaService( | |
| { | ||
| schema.ApplyNullabilityContextInfo(jsonPropertyInfo); | ||
| } | ||
| if (context.TypeInfo.Type.GetCustomAttributes(inherit: false).OfType<DescriptionAttribute>().LastOrDefault() is { } typeDescriptionAttribute) | ||
| { | ||
| schema[OpenApiSchemaKeywords.DescriptionKeyword] = typeDescriptionAttribute.Description; | ||
| } | ||
| if (context.PropertyInfo is { AttributeProvider: { } attributeProvider }) | ||
| { | ||
| if (attributeProvider.GetCustomAttributes(inherit: false).OfType<ValidationAttribute>() is { } validationAttributes) | ||
| var propertyAttributes = attributeProvider.GetCustomAttributes(inherit: false); | ||
| if (propertyAttributes.OfType<ValidationAttribute>() is { } validationAttributes) | ||
| { | ||
| schema.ApplyValidationAttributes(validationAttributes); | ||
| } | ||
| if (attributeProvider.GetCustomAttributes(inherit: false).OfType<DefaultValueAttribute>().LastOrDefault() is DefaultValueAttribute defaultValueAttribute) | ||
| if (propertyAttributes.OfType<DefaultValueAttribute>().LastOrDefault() is { } defaultValueAttribute) | ||
| { | ||
| schema.ApplyDefaultValue(defaultValueAttribute.Value, context.TypeInfo); | ||
| } | ||
| if (attributeProvider.GetCustomAttributes(inherit: false).OfType<DescriptionAttribute>().LastOrDefault() is DescriptionAttribute descriptionAttribute) | ||
| var isInlinedSchema = schema[OpenApiConstants.SchemaId] is null; | ||
| if (isInlinedSchema) | ||
| There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. | ||
| { | ||
| if (propertyAttributes.OfType<DescriptionAttribute>().LastOrDefault() is { } descriptionAttribute) | ||
| { | ||
| schema[OpenApiSchemaKeywords.DescriptionKeyword] = descriptionAttribute.Description; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| schema[OpenApiSchemaKeywords.DescriptionKeyword] = descriptionAttribute.Description; | ||
| if (propertyAttributes.OfType<DescriptionAttribute>().LastOrDefault() is { } descriptionAttribute) | ||
| { | ||
| schema[OpenApiConstants.RefDescriptionAnnotation] = descriptionAttribute.Description; | ||
| } | ||
| } | ||
| } | ||
|  | ||
|  | ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|  | ||
| using System.ComponentModel; | ||
| using System.Net.Http; | ||
| using Microsoft.AspNetCore.Builder; | ||
| using Microsoft.AspNetCore.OpenApi; | ||
| using Microsoft.AspNetCore.Routing; | ||
|  | ||
| public partial class OpenApiSchemaServiceTests : OpenApiDocumentServiceTestBase | ||
| { | ||
| [Fact] | ||
| public async Task SchemaDescriptions_HandlesSchemaReferences() | ||
| { | ||
| // Arrange | ||
| var builder = CreateBuilder(); | ||
|  | ||
| // Act | ||
| builder.MapPost("/", (DescribedReferencesDto dto) => { }); | ||
|  | ||
| // Assert | ||
| await VerifyOpenApiDocument(builder, document => | ||
| { | ||
| var operation = document.Paths["/"].Operations[HttpMethod.Post]; | ||
| var requestBody = operation.RequestBody; | ||
|  | ||
| Assert.NotNull(requestBody); | ||
| var content = Assert.Single(requestBody.Content); | ||
| Assert.Equal("application/json", content.Key); | ||
| Assert.NotNull(content.Value.Schema); | ||
| var schema = content.Value.Schema; | ||
| Assert.Equal(JsonSchemaType.Object, schema.Type); | ||
| Assert.Collection(schema.Properties, | ||
| property => | ||
| { | ||
| Assert.Equal("child1", property.Key); | ||
| var reference = Assert.IsType<OpenApiSchemaReference>(property.Value); | ||
| Assert.Equal("Property: DescribedReferencesDto.Child1", reference.Reference.Description); | ||
| }, | ||
| property => | ||
| { | ||
| Assert.Equal("child2", property.Key); | ||
| var reference = Assert.IsType<OpenApiSchemaReference>(property.Value); | ||
| Assert.Equal("Property: DescribedReferencesDto.Child2", reference.Reference.Description); | ||
| }, | ||
| property => | ||
| { | ||
| Assert.Equal("childNoDescription", property.Key); | ||
| var reference = Assert.IsType<OpenApiSchemaReference>(property.Value); | ||
| Assert.Null(reference.Reference.Description); | ||
| }); | ||
|  | ||
| var referencedSchema = document.Components.Schemas["DescribedChildDto"]; | ||
| Assert.Equal("Class: DescribedChildDto", referencedSchema.Description); | ||
| }); | ||
|  | ||
| } | ||
|  | ||
| [Description("Class: DescribedReferencesDto")] | ||
| public class DescribedReferencesDto | ||
| { | ||
| [Description("Property: DescribedReferencesDto.Child1")] | ||
| public DescribedChildDto Child1 { get; set; } | ||
|  | ||
| [Description("Property: DescribedReferencesDto.Child2")] | ||
| public DescribedChildDto Child2 { get; set; } | ||
|  | ||
| public DescribedChildDto ChildNoDescription { get; set; } | ||
| } | ||
|  | ||
| [Description("Class: DescribedChildDto")] | ||
| public class DescribedChildDto | ||
| { | ||
| [Description("Property: DescribedChildDto.ChildValue")] | ||
| public string ChildValue { get; set; } | ||
| } | ||
|  | ||
| [Fact] | ||
| public async Task SchemaDescriptions_HandlesInlinedSchemas() | ||
| { | ||
| // Arrange | ||
| var builder = CreateBuilder(); | ||
|  | ||
| var options = new OpenApiOptions(); | ||
| var originalCreateSchemaReferenceId = options.CreateSchemaReferenceId; | ||
| options.CreateSchemaReferenceId = (x) => x.Type == typeof(DescribedInlinedDto) ? null : originalCreateSchemaReferenceId(x); | ||
|  | ||
| // Act | ||
| builder.MapPost("/", (DescribedInlinedSchemasDto dto) => { }); | ||
|  | ||
| // Assert | ||
| await VerifyOpenApiDocument(builder, options, document => | ||
| { | ||
| var operation = document.Paths["/"].Operations[HttpMethod.Post]; | ||
| var requestBody = operation.RequestBody; | ||
|  | ||
| Assert.NotNull(requestBody); | ||
| var content = Assert.Single(requestBody.Content); | ||
| Assert.Equal("application/json", content.Key); | ||
| Assert.NotNull(content.Value.Schema); | ||
| var schema = content.Value.Schema; | ||
| Assert.Equal(JsonSchemaType.Object, schema.Type); | ||
| Assert.Collection(schema.Properties, | ||
| property => | ||
| { | ||
| Assert.Equal("inlined1", property.Key); | ||
| var inlinedSchema = Assert.IsType<OpenApiSchema>(property.Value); | ||
| Assert.Equal("Property: DescribedInlinedSchemasDto.Inlined1", inlinedSchema.Description); | ||
| }, | ||
| property => | ||
| { | ||
| Assert.Equal("inlined2", property.Key); | ||
| var inlinedSchema = Assert.IsType<OpenApiSchema>(property.Value); | ||
| Assert.Equal("Property: DescribedInlinedSchemasDto.Inlined2", inlinedSchema.Description); | ||
| }, | ||
| property => | ||
| { | ||
| Assert.Equal("inlinedNoDescription", property.Key); | ||
| var inlinedSchema = Assert.IsType<OpenApiSchema>(property.Value); | ||
| Assert.Equal("Class: DescribedInlinedDto", inlinedSchema.Description); | ||
| }); | ||
| }); | ||
| } | ||
|  | ||
| [Description("Class: DescribedInlinedSchemasDto")] | ||
| public class DescribedInlinedSchemasDto | ||
| { | ||
| [Description("Property: DescribedInlinedSchemasDto.Inlined1")] | ||
| public DescribedInlinedDto Inlined1 { get; set; } | ||
|  | ||
| [Description("Property: DescribedInlinedSchemasDto.Inlined2")] | ||
| public DescribedInlinedDto Inlined2 { get; set; } | ||
|  | ||
| public DescribedInlinedDto InlinedNoDescription { get; set; } | ||
| } | ||
|  | ||
| [Description("Class: DescribedInlinedDto")] | ||
| public class DescribedInlinedDto | ||
| { | ||
| [Description("Property: DescribedInlinedDto.ChildValue")] | ||
| public string ChildValue { get; set; } | ||
| } | ||
| } | 
Uh oh!
There was an error while loading. Please reload this page.