Skip to content

Fixup OpenAPI displayed param type when using TryParse #36526

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.

Sign up for GitHub

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
Sep 16, 2021
Merged

Fixup OpenAPI displayed param type when using TryParse #36526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23369c7
Fixup OpenAPI displayed param type when using TryParse
BrennanConroy Sep 15, 2021
420a9c1
Update src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionPro…
BrennanConroy Sep 15, 2021
9cd135b
nullability
BrennanConroy Sep 15, 2021
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
25 changes: 14 additions & 11 deletions src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string

private ApiParameterDescription? CreateApiParameterDescription(ParameterInfo parameter, RoutePattern pattern)
{
var (source, name, allowEmpty) = GetBindingSourceAndName(parameter, pattern);
var (source, name, allowEmpty, paramType) = GetBindingSourceAndName(parameter, pattern);

// Services are ignored because they are not request parameters.
// We ignore/skip body parameter because the value will be retrieved from the IAcceptsMetadata.
Expand All @@ -165,7 +165,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string
return new ApiParameterDescription
{
Name = name,
ModelMetadata = CreateModelMetadata(parameter.ParameterType),
ModelMetadata = CreateModelMetadata(paramType),
Source = source,
DefaultValue = parameter.DefaultValue,
Type = parameter.ParameterType,
Expand All @@ -184,25 +184,25 @@ private static ParameterDescriptor CreateParameterDescriptor(ParameterInfo param

// TODO: Share more of this logic with RequestDelegateFactory.CreateArgument(...) using RequestDelegateFactoryUtilities
// which is shared source.
private (BindingSource, string, bool) GetBindingSourceAndName(ParameterInfo parameter, RoutePattern pattern)
private (BindingSource, string, bool, Type) GetBindingSourceAndName(ParameterInfo parameter, RoutePattern pattern)
{
var attributes = parameter.GetCustomAttributes();

if (attributes.OfType<IFromRouteMetadata>().FirstOrDefault() is { } routeAttribute)
{
return (BindingSource.Path, routeAttribute.Name ?? parameter.Name ?? string.Empty, false);
return (BindingSource.Path, routeAttribute.Name ?? parameter.Name ?? string.Empty, false, parameter.ParameterType);
}
else if (attributes.OfType<IFromQueryMetadata>().FirstOrDefault() is { } queryAttribute)
{
return (BindingSource.Query, queryAttribute.Name ?? parameter.Name ?? string.Empty, false);
return (BindingSource.Query, queryAttribute.Name ?? parameter.Name ?? string.Empty, false, parameter.ParameterType);
}
else if (attributes.OfType<IFromHeaderMetadata>().FirstOrDefault() is { } headerAttribute)
{
return (BindingSource.Header, headerAttribute.Name ?? parameter.Name ?? string.Empty, false);
return (BindingSource.Header, headerAttribute.Name ?? parameter.Name ?? string.Empty, false, parameter.ParameterType);
}
else if (attributes.OfType<IFromBodyMetadata>().FirstOrDefault() is { } fromBodyAttribute)
{
return (BindingSource.Body, parameter.Name ?? string.Empty, fromBodyAttribute.AllowEmpty);
return (BindingSource.Body, parameter.Name ?? string.Empty, fromBodyAttribute.AllowEmpty, parameter.ParameterType);
}
else if (parameter.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType)) ||
parameter.ParameterType == typeof(HttpContext) ||
Expand All @@ -213,23 +213,26 @@ private static ParameterDescriptor CreateParameterDescriptor(ParameterInfo param
ParameterBindingMethodCache.HasBindAsyncMethod(parameter) ||
_serviceProviderIsService?.IsService(parameter.ParameterType) == true)
{
return (BindingSource.Services, parameter.Name ?? string.Empty, false);
return (BindingSource.Services, parameter.Name ?? string.Empty, false, parameter.ParameterType);
}
else if (parameter.ParameterType == typeof(string) || ParameterBindingMethodCache.HasTryParseMethod(parameter))
{
// complex types will display as strings since they use custom parsing via TryParse on a string
var displayType = !parameter.ParameterType.IsPrimitive && Nullable.GetUnderlyingType(parameter.ParameterType)?.IsPrimitive != true
? typeof(string) : parameter.ParameterType;
// Path vs query cannot be determined by RequestDelegateFactory at startup currently because of the layering, but can be done here.
if (parameter.Name is { } name && pattern.GetParameter(name) is not null)
{
return (BindingSource.Path, name, false);
return (BindingSource.Path, name, false, displayType);
}
else
{
return (BindingSource.Query, parameter.Name ?? string.Empty, false);
return (BindingSource.Query, parameter.Name ?? string.Empty, false, displayType);
}
}
else
{
return (BindingSource.Body, parameter.Name ?? string.Empty, false);
return (BindingSource.Body, parameter.Name ?? string.Empty, false, parameter.ParameterType);
}
}

Expand Down
52 changes: 50 additions & 2 deletions src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,19 +266,61 @@ static void AssertPathParameter(ApiDescription apiDescription)
}

[Fact]
public void AddsFromRouteParameterAsPathWithCustomType()
public void AddsFromRouteParameterAsPathWithCustomClassWithTryParse()
{
static void AssertPathParameter(ApiDescription apiDescription)
{
var param = Assert.Single(apiDescription.ParameterDescriptions);
Assert.Equal(typeof(TryParseStringRecord), param.Type);
Assert.Equal(typeof(TryParseStringRecord), param.ModelMetadata.ModelType);
Assert.Equal(typeof(string), param.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Path, param.Source);
}

AssertPathParameter(GetApiDescription((TryParseStringRecord foo) => { }, "/{foo}"));
}

[Fact]
public void AddsFromRouteParameterAsPathWithPrimitiveType()
{
static void AssertPathParameter(ApiDescription apiDescription)
{
var param = Assert.Single(apiDescription.ParameterDescriptions);
Assert.Equal(typeof(int), param.Type);
Assert.Equal(typeof(int), param.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Path, param.Source);
}

AssertPathParameter(GetApiDescription((int foo) => { }, "/{foo}"));
}

[Fact]
public void AddsFromRouteParameterAsPathWithNullablePrimitiveType()
{
static void AssertPathParameter(ApiDescription apiDescription)
{
var param = Assert.Single(apiDescription.ParameterDescriptions);
Assert.Equal(typeof(int?), param.Type);
Assert.Equal(typeof(int?), param.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Path, param.Source);
}

AssertPathParameter(GetApiDescription((int? foo) => { }, "/{foo}"));
}

[Fact]
public void AddsFromRouteParameterAsPathWithStructTypeWithTryParse()
{
static void AssertPathParameter(ApiDescription apiDescription)
{
var param = Assert.Single(apiDescription.ParameterDescriptions);
Assert.Equal(typeof(TryParseStringRecordStruct), param.Type);
Assert.Equal(typeof(string), param.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Path, param.Source);
}

AssertPathParameter(GetApiDescription((TryParseStringRecordStruct foo) => { }, "/{foo}"));
}

[Fact]
public void AddsFromQueryParameterAsQuery()
{
Expand Down Expand Up @@ -918,6 +960,12 @@ public static bool TryParse(string value, out TryParseStringRecord result) =>
throw new NotImplementedException();
}

private record struct TryParseStringRecordStruct(int Value)
{
public static bool TryParse(string value, out TryParseStringRecordStruct result) =>
throw new NotImplementedException();
}

private record BindAsyncRecord(int Value)
{
public static ValueTask<BindAsyncRecord> BindAsync(HttpContext context, ParameterInfo parameter) =>
Expand Down