Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/OpenApi/src/Extensions/JsonNodeSchemaExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,11 @@ internal static void ApplyParameterInfo(this JsonNode schema, ApiParameterDescri
var attributes = validations.OfType<ValidationAttribute>();
schema.ApplyValidationAttributes(attributes);
}
if (parameterDescription.ModelMetadata is Mvc.ModelBinding.Metadata.DefaultModelMetadata { Attributes.PropertyAttributes.Count: > 0 } metadata &&
metadata.Attributes.PropertyAttributes.OfType<DefaultValueAttribute>().LastOrDefault() is { } metadataDefaultValueAttribute)
{
schema.ApplyDefaultValue(metadataDefaultValueAttribute.Value, jsonTypeInfo);
}
if (parameterDescription.ParameterDescriptor is IParameterInfoParameterDescriptor { ParameterInfo: { } parameterInfo })
{
if (parameterInfo.HasDefaultValue)
Expand Down
21 changes: 16 additions & 5 deletions src/OpenApi/src/Services/OpenApiDocumentService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -494,11 +494,22 @@ private static bool IsRequired(ApiParameterDescription parameter)
}

// Apply [Description] attributes on the parameter to the top-level OpenApiParameter object and not the schema.
private static string? GetParameterDescriptionFromAttribute(ApiParameterDescription parameter) =>
parameter.ParameterDescriptor is IParameterInfoParameterDescriptor { ParameterInfo: { } parameterInfo } &&
parameterInfo.GetCustomAttributes().OfType<DescriptionAttribute>().LastOrDefault() is { } descriptionAttribute ?
descriptionAttribute.Description :
null;
private static string? GetParameterDescriptionFromAttribute(ApiParameterDescription parameter)
{
if (parameter.ParameterDescriptor is IParameterInfoParameterDescriptor { ParameterInfo: { } parameterInfo } &&
parameterInfo.GetCustomAttributes<DescriptionAttribute>().LastOrDefault() is { } parameterDescription)
{
return parameterDescription.Description;
}

if (parameter.ModelMetadata is Mvc.ModelBinding.Metadata.DefaultModelMetadata { Attributes.PropertyAttributes.Count: > 0 } metadata &&
metadata.Attributes.PropertyAttributes.OfType<DescriptionAttribute>().LastOrDefault() is { } propertyDescription)
{
return propertyDescription.Description;
}
Comment on lines +505 to +509
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure this is right, as it feels wrong to depend on a specific model metadata implementation, but I couldn't immediately see an obvious interface-y/abstract way to query the same information and this was "right there" in the debugger.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for practical cases this is sufficient.

The APIs will only ever end up consuming two metadata types:

  • EndpointModelMetadata which is used for minimal API
  • DefaultModelMetadata for MVC

The minimal API cases end up getting handled by the IParameterInfoParameterDescriptor because of the way we map everything, including [AsParameters] to parameter descriptions via things like PropertyAsParameterInfo.

This is also why the bug reports that we've received specifically site this gap in controllers and not minimal APIs.

So although it looks werid and is awakward, I think this is the correct way give the fact that MVC relies on model metadata and minimal API doesn't.


return null;
}

private async Task<OpenApiRequestBody?> GetRequestBodyAsync(OpenApiDocument document, ApiDescription description, IServiceProvider scopedServiceProvider, IOpenApiSchemaTransformer[] schemaTransformers, CancellationToken cancellationToken)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ public ControllerActionDescriptor CreateActionDescriptor(string methodName = nul

action.AttributeRouteInfo = new()
{
Template = action.MethodInfo.GetCustomAttribute<RouteAttribute>()?.Template,
Template = action.MethodInfo.GetCustomAttribute<RouteAttribute>()?.Template ?? string.Empty,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is to make this assertion happy:

Debug.Assert(apiDescription.RelativePath != null, "Relative path cannot be null.");

Name = action.MethodInfo.GetCustomAttribute<RouteAttribute>()?.Name,
Order = action.MethodInfo.GetCustomAttribute<RouteAttribute>()?.Order ?? 0,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,86 @@ await VerifyOpenApiDocument(builder, document =>
});
}

[Fact]
public async Task GetOpenApiParameters_HandlesAsParametersParametersWithDescriptionAttribute()
{
// Arrange
var builder = CreateBuilder();

// Act
builder.MapGet("/api", ([AsParameters] FromQueryModel model) => { });

// Assert
await VerifyOpenApiDocument(builder, document =>
{
var operation = document.Paths["/api"].Operations[HttpMethod.Get];
Assert.Contains(operation.Parameters, actualMemory => actualMemory.Name == "id" && actualMemory.Description == "The ID of the entity");
});
}

[Fact]
public async Task GetOpenApiParameters_HandlesFromQueryParametersWithDescriptionAttribute()
{
// Arrange
var actionDescriptor = CreateActionDescriptor(nameof(TestFromQueryController.GetWithFromQueryDto), typeof(TestFromQueryController));

// Assert
await VerifyOpenApiDocument(actionDescriptor, document =>
{
var operation = document.Paths["/"].Operations[HttpMethod.Get];
Assert.Contains(operation.Parameters, actualMemory => actualMemory.Name == "id" && actualMemory.Description == "The ID of the entity");
});
}

[Fact]
public async Task GetOpenApiParameters_HandlesAsParametersParametersWithDefaultValueAttribute()
{
// Arrange
var builder = CreateBuilder();

// Act
builder.MapGet("/api", ([AsParameters] FromQueryModel model) => { });

// Assert
await VerifyOpenApiDocument(builder, document =>
{
var operation = document.Paths["/api"].Operations[HttpMethod.Get];
Assert.Contains(
operation.Parameters,
actualMemory =>
{
return actualMemory.Name == "limit" &&
actualMemory.Schema != null &&
actualMemory.Schema.Default != null &&
actualMemory.Schema.Default.GetValueKind() == JsonValueKind.Number &&
actualMemory.Schema.Default.GetValue<int>() == 20;
});
});
}

[Fact]
public async Task GetOpenApiParameters_HandlesFromQueryParametersWithDefaultValueAttribute()
{
// Arrange
var actionDescriptor = CreateActionDescriptor(nameof(TestFromQueryController.GetWithFromQueryDto), typeof(TestFromQueryController));

// Assert
await VerifyOpenApiDocument(actionDescriptor, document =>
{
var operation = document.Paths["/"].Operations[HttpMethod.Get];
Assert.Contains(
operation.Parameters,
actualMemory =>
{
return actualMemory.Name == "limit" &&
actualMemory.Schema != null &&
actualMemory.Schema.Default != null &&
actualMemory.Schema.Default.GetValueKind() == JsonValueKind.Number &&
actualMemory.Schema.Default.GetValue<int>() == 20;
});
});
}

[Route("/api/{id}/{date}")]
private void AcceptsParametersInModel(RouteParamsContainer model) { }

Expand Down Expand Up @@ -809,4 +889,28 @@ public override void Write(Utf8JsonWriter writer, EnumArrayType value, JsonSeria
writer.WriteEndObject();
}
}

[ApiController]
[Route("[controller]/[action]")]
private class TestFromQueryController : ControllerBase
{
[HttpGet]
public Task<IActionResult> GetWithFromQueryDto([FromQuery] FromQueryModel query)
{
return Task.FromResult<IActionResult>(Ok());
}
}

[Description("A query model.")]
private record FromQueryModel
{
[Description("The ID of the entity")]
[FromQuery(Name = "id")]
public int Id { get; set; }

[Description("The maximum number of results")]
[FromQuery(Name = "limit")]
[DefaultValue(20)]
public int Limit { get; set; }
}
}
Loading