Skip to content

Fix ModelMetadata for TryParse-parameters in ApiExplorer #58350

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

Merged
merged 3 commits into from
Oct 11, 2024
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
49 changes: 38 additions & 11 deletions src/OpenApi/src/Services/OpenApiDocumentService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -403,16 +403,6 @@ private async Task<OpenApiResponse> GetResponseAsync(
continue;
}

// MVC's ModelMetadata layer will set ApiParameterDescription.Type to string when the parameter
// is a parsable or convertible type. In this case, we want to use the actual model type
// to generate the schema instead of the string type.
var targetType = parameter.Type == typeof(string) && parameter.ModelMetadata.ModelType != parameter.Type
? parameter.ModelMetadata.ModelType
: parameter.Type;
// If the type is null, then we're dealing with an inert
// route parameter that does not define a specific parameter type in the
// route handler or in the response. In this case, we default to a string schema.
targetType ??= typeof(string);
var openApiParameter = new OpenApiParameter
{
Name = parameter.Name,
Expand All @@ -424,7 +414,7 @@ private async Task<OpenApiResponse> GetResponseAsync(
_ => throw new InvalidOperationException($"Unsupported parameter source: {parameter.Source.Id}")
},
Required = IsRequired(parameter),
Schema = await _componentService.GetOrCreateSchemaAsync(targetType, scopedServiceProvider, schemaTransformers, parameter, cancellationToken: cancellationToken),
Schema = await _componentService.GetOrCreateSchemaAsync(GetTargetType(description, parameter), scopedServiceProvider, schemaTransformers, parameter, cancellationToken: cancellationToken),
Description = GetParameterDescriptionFromAttribute(parameter)
};

Expand Down Expand Up @@ -675,4 +665,41 @@ private async Task<OpenApiRequestBody> GetJsonRequestBody(

return requestBody;
}

/// <remarks>
/// This method is used to determine the target type for a given parameter. The target type
/// is the actual type that should be used to generate the schema for the parameter. This is
/// necessary because MVC's ModelMetadata layer will set ApiParameterDescription.Type to string
/// when the parameter is a parsable or convertible type. In this case, we want to use the actual
/// model type to generate the schema instead of the string type.
/// </remarks>
/// <remarks>
/// This method will also check if no target type was resolved from the <see cref="ApiParameterDescription"/>
/// and default to a string schema. This will happen if we are dealing with an inert route parameter
/// that does not define a specific parameter type in the route handler or in the response.
/// </remarks>
private static Type GetTargetType(ApiDescription description, ApiParameterDescription parameter)
{
var bindingMetadata = description.ActionDescriptor.EndpointMetadata
.OfType<IParameterBindingMetadata>()
.SingleOrDefault(metadata => metadata.Name == parameter.Name);
var parameterType = parameter.Type is not null
? Nullable.GetUnderlyingType(parameter.Type) ?? parameter.Type
: parameter.Type;

// parameter.Type = typeof(string)
// parameter.ModelMetadata.Type = typeof(TEnum)
var requiresModelMetadataFallbackForEnum = parameterType == typeof(string)
&& parameter.ModelMetadata.ModelType != parameter.Type
&& parameter.ModelMetadata.ModelType.IsEnum;
// Enums are exempt because we want to set the OpenApiSchema.Enum field when feasible.
// parameter.Type = typeof(TEnum), typeof(TypeWithTryParse)
// parameter.ModelMetadata.Type = typeof(string)
var hasTryParse = bindingMetadata?.HasTryParse == true && parameterType is not null && !parameterType.IsEnum;
var targetType = requiresModelMetadataFallbackForEnum || hasTryParse
? parameter.ModelMetadata.ModelType
: parameter.Type;
targetType ??= typeof(string);
return targetType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,8 @@ public async Task SupportsParameterWithEnumType(bool useAction)
if (!useAction)
{
var builder = CreateBuilder();
builder.MapGet("/api/with-enum", (ItemStatus status) => status);
builder.MapGet("/api/with-enum", (Status status) => status);
await VerifyOpenApiDocument(builder, AssertOpenApiDocument);
}
else
{
Expand Down Expand Up @@ -583,15 +584,7 @@ static void AssertOpenApiDocument(OpenApiDocument document)
}

[Route("/api/with-enum")]
private ItemStatus GetItemStatus([FromQuery] ItemStatus status) => status;

[JsonConverter(typeof(JsonStringEnumConverter<ItemStatus>))]
internal enum ItemStatus
{
Pending = 0,
Approved = 1,
Rejected = 2,
}
private Status GetItemStatus([FromQuery] Status status) => status;

[Fact]
public async Task SupportsMvcActionWithAmbientRouteParameter()
Expand All @@ -610,4 +603,63 @@ await VerifyOpenApiDocument(action, document =>

[Route("/api/with-ambient-route-param/{versionId}")]
private void AmbientRouteParameter() { }

[Theory]
[InlineData(false)]
[InlineData(true)]
public async Task SupportsRouteParameterWithCustomTryParse(bool useAction)
{
// Arrange
var builder = CreateBuilder();

// Act
if (!useAction)
{
builder.MapGet("/api/{student}", (Student student) => student);
await VerifyOpenApiDocument(builder, AssertOpenApiDocument);
}
else
{
var action = CreateActionDescriptor(nameof(GetStudent));
await VerifyOpenApiDocument(action, AssertOpenApiDocument);
}

// Assert
static void AssertOpenApiDocument(OpenApiDocument document)
{
// Parameter is a plain-old string when it comes from the route or query
var operation = document.Paths["/api/{student}"].Operations[OperationType.Get];
var parameter = Assert.Single(operation.Parameters);
Assert.Equal("string", parameter.Schema.Type);

// Type is fully serialized in the response
var response = Assert.Single(operation.Responses).Value;
Assert.True(response.Content.TryGetValue("application/json", out var mediaType));
var schema = mediaType.Schema.GetEffective(document);
Assert.Equal("object", schema.Type);
Assert.Collection(schema.Properties, property =>
{
Assert.Equal("name", property.Key);
Assert.Equal("string", property.Value.Type);
});
}
}

[Route("/api/{student}")]
private Student GetStudent(Student student) => student;

public record Student(string Name)
{
public static bool TryParse(string value, out Student result)
{
if (value is null)
{
result = null;
return false;
}

result = new Student(value);
return true;
}
}
}
Loading