Skip to content

Set IsRequired on ApiDescriptions for endpoints #35233

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 2 commits into from
Aug 11, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ internal class EndpointMetadataApiDescriptionProvider : IApiDescriptionProvider
private readonly IHostEnvironment _environment;
private readonly IServiceProviderIsService? _serviceProviderIsService;
private readonly TryParseMethodCache TryParseMethodCache = new();
private readonly NullabilityInfoContext NullabilityContext = new();

// Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider for no particular reason.
public int Order => -1100;
Expand Down Expand Up @@ -132,45 +133,50 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string

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

// Services are ignored because they are not request parameters.
if (source == BindingSource.Services)
{
return null;
}

// Determine the "requiredness" based on nullability, default value or if allowEmpty is set
var nullability = NullabilityContext.Create(parameter);
var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable || allowEmpty;

return new ApiParameterDescription
{
Name = name,
ModelMetadata = CreateModelMetadata(parameter.ParameterType),
Source = source,
DefaultValue = parameter.DefaultValue,
Type = parameter.ParameterType,
IsRequired = !isOptional
};
}

// TODO: Share more of this logic with RequestDelegateFactory.CreateArgument(...) using RequestDelegateFactoryUtilities
// which is shared source.
private (BindingSource, string) GetBindingSourceAndName(ParameterInfo parameter, RoutePattern pattern)
private (BindingSource, string, bool) 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);
return (BindingSource.Path, routeAttribute.Name ?? parameter.Name ?? string.Empty, false);
}
else if (attributes.OfType<IFromQueryMetadata>().FirstOrDefault() is { } queryAttribute)
{
return (BindingSource.Query, queryAttribute.Name ?? parameter.Name ?? string.Empty);
return (BindingSource.Query, queryAttribute.Name ?? parameter.Name ?? string.Empty, false);
}
else if (attributes.OfType<IFromHeaderMetadata>().FirstOrDefault() is { } headerAttribute)
{
return (BindingSource.Header, headerAttribute.Name ?? parameter.Name ?? string.Empty);
return (BindingSource.Header, headerAttribute.Name ?? parameter.Name ?? string.Empty, false);
}
else if (parameter.CustomAttributes.Any(a => typeof(IFromBodyMetadata).IsAssignableFrom(a.AttributeType)))
else if (attributes.OfType<IFromBodyMetadata>().FirstOrDefault() is { } fromBodyAttribute)
{
return (BindingSource.Body, parameter.Name ?? string.Empty);
return (BindingSource.Body, parameter.Name ?? string.Empty, fromBodyAttribute.AllowEmpty);
}
else if (parameter.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType)) ||
parameter.ParameterType == typeof(HttpContext) ||
Expand All @@ -180,23 +186,23 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string
parameter.ParameterType == typeof(CancellationToken) ||
_serviceProviderIsService?.IsService(parameter.ParameterType) == true)
{
return (BindingSource.Services, parameter.Name ?? string.Empty);
return (BindingSource.Services, parameter.Name ?? string.Empty, false);
}
else if (parameter.ParameterType == typeof(string) || TryParseMethodCache.HasTryParseMethod(parameter))
{
// 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);
return (BindingSource.Path, name, false);
}
else
{
return (BindingSource.Query, parameter.Name ?? string.Empty);
return (BindingSource.Query, parameter.Name ?? string.Empty, false);
}
}
else
{
return (BindingSource.Body, parameter.Name ?? string.Empty);
return (BindingSource.Body, parameter.Name ?? string.Empty, false);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,28 +76,31 @@ static void AssertJsonRequestFormat(ApiDescription apiDescription)
[Fact]
public void AddsRequestFormatFromMetadata()
{
static void AssertustomRequestFormat(ApiDescription apiDescription)
static void AssertCustomRequestFormat(ApiDescription apiDescription)
{
var requestFormat = Assert.Single(apiDescription.SupportedRequestFormats);
Assert.Equal("application/custom", requestFormat.MediaType);
Assert.Null(requestFormat.Formatter);
}

AssertustomRequestFormat(GetApiDescription(
AssertCustomRequestFormat(GetApiDescription(
[Consumes("application/custom")]
(InferredJsonClass fromBody) => { }));
(InferredJsonClass fromBody) =>
{ }));

AssertustomRequestFormat(GetApiDescription(
AssertCustomRequestFormat(GetApiDescription(
[Consumes("application/custom")]
([FromBody] int fromBody) => { }));
([FromBody] int fromBody) =>
{ }));
}

[Fact]
public void AddsMultipleRequestFormatsFromMetadata()
{
var apiDescription = GetApiDescription(
[Consumes("application/custom0", "application/custom1")]
(InferredJsonClass fromBody) => { });
(InferredJsonClass fromBody) =>
{ });

Assert.Equal(2, apiDescription.SupportedRequestFormats.Count);

Expand Down Expand Up @@ -167,8 +170,8 @@ public void AddsResponseFormatFromMetadata()
{
var apiDescription = GetApiDescription(
[ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created)]
[Produces("application/custom")]
() => new InferredJsonClass());
[Produces("application/custom")]
() => new InferredJsonClass());

var responseType = Assert.Single(apiDescription.SupportedResponseTypes);

Expand All @@ -185,8 +188,8 @@ public void AddsMultipleResponseFormatsFromMetadataWithPoco()
{
var apiDescription = GetApiDescription(
[ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created)]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
() => new InferredJsonClass());
[ProducesResponseType(StatusCodes.Status400BadRequest)]
() => new InferredJsonClass());

Assert.Equal(2, apiDescription.SupportedResponseTypes.Count);

Expand Down Expand Up @@ -214,8 +217,8 @@ public void AddsMultipleResponseFormatsFromMetadataWithIResult()
{
var apiDescription = GetApiDescription(
[ProducesResponseType(typeof(InferredJsonClass), StatusCodes.Status201Created)]
[ProducesResponseType(StatusCodes.Status400BadRequest)]
() => Results.Ok(new InferredJsonClass()));
[ProducesResponseType(StatusCodes.Status400BadRequest)]
() => Results.Ok(new InferredJsonClass()));

Assert.Equal(2, apiDescription.SupportedResponseTypes.Count);

Expand Down Expand Up @@ -324,18 +327,68 @@ public void AddsMultipleParameters()
Assert.Equal(typeof(int), fooParam.Type);
Assert.Equal(typeof(int), fooParam.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Path, fooParam.Source);
Assert.True(fooParam.IsRequired);

var barParam = apiDescription.ParameterDescriptions[1];
Assert.Equal(typeof(int), barParam.Type);
Assert.Equal(typeof(int), barParam.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Query, barParam.Source);
Assert.True(barParam.IsRequired);

var fromBodyParam = apiDescription.ParameterDescriptions[2];
Assert.Equal(typeof(InferredJsonClass), fromBodyParam.Type);
Assert.Equal(typeof(InferredJsonClass), fromBodyParam.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Body, fromBodyParam.Source);
Assert.True(fromBodyParam.IsRequired);
}

[Fact]
public void TestParameterIsRequired()
{
var apiDescription = GetApiDescription(([FromRoute] int foo, int? bar) => { });
Assert.Equal(2, apiDescription.ParameterDescriptions.Count);

var fooParam = apiDescription.ParameterDescriptions[0];
Assert.Equal(typeof(int), fooParam.Type);
Assert.Equal(typeof(int), fooParam.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Path, fooParam.Source);
Assert.True(fooParam.IsRequired);

var barParam = apiDescription.ParameterDescriptions[1];
Assert.Equal(typeof(int?), barParam.Type);
Assert.Equal(typeof(int?), barParam.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Query, barParam.Source);
Assert.False(barParam.IsRequired);
}

#nullable enable

[Fact]
public void TestIsRequiredFromBody()
{
var apiDescription0 = GetApiDescription(([FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] InferredJsonClass fromBody) => { });
var apiDescription1 = GetApiDescription((InferredJsonClass? fromBody) => { });
Assert.Equal(1, apiDescription0.ParameterDescriptions.Count);
Assert.Equal(1, apiDescription1.ParameterDescriptions.Count);

var fromBodyParam0 = apiDescription0.ParameterDescriptions[0];
Assert.Equal(typeof(InferredJsonClass), fromBodyParam0.Type);
Assert.Equal(typeof(InferredJsonClass), fromBodyParam0.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Body, fromBodyParam0.Source);
Assert.False(fromBodyParam0.IsRequired);

var fromBodyParam1 = apiDescription1.ParameterDescriptions[0];
Assert.Equal(typeof(InferredJsonClass), fromBodyParam1.Type);
Assert.Equal(typeof(InferredJsonClass), fromBodyParam1.ModelMetadata.ModelType);
Assert.Equal(BindingSource.Body, fromBodyParam1.Source);
Assert.False(fromBodyParam1.IsRequired);
}

// This is necessary for TestIsRequiredFromBody to pass until https://github.com/dotnet/roslyn/issues/55254 is resolved.
private object RandomMethod() => throw new NotImplementedException();

#nullable disable

[Fact]
public void AddsDisplayNameFromRouteEndpoint()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<LangVersion>Preview</LangVersion>
<Features>$(Features.Replace('nullablePublicOnly', '')</Features>
</PropertyGroup>

<ItemGroup>
Expand Down