Skip to content

Commit

Permalink
Fix ModelMetadata for TryParse-parameters in ApiExplorer (#58350)
Browse files Browse the repository at this point in the history
* Fix ModelMetadata for TryParse-parameters in ApiExplorer

* Add tests and update code comments

* Update src/OpenApi/src/Services/OpenApiDocumentService.cs
  • Loading branch information
captainsafia authored Oct 11, 2024
1 parent 54c0cc8 commit 579b3fd
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 21 deletions.
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;
}
}
}

0 comments on commit 579b3fd

Please sign in to comment.