diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index b1f173440043..af41f16d6ab2 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -7075,6 +7075,158 @@ public void Create_Populates_EndpointBuilderWithRequestDelegateAndMetadata() Assert.Same(options.EndpointBuilder.Metadata, result.EndpointMetadata); } + private class ParameterListRequiredStringFromDifferentSources + { + public HttpContext? HttpContext { get; set; } + + [FromRoute] + public required string RequiredRouteParam { get; set; } + + [FromQuery] + public required string RequiredQueryParam { get; set; } + + [FromHeader] + public required string RequiredHeaderParam { get; set; } + } + + [Fact] + public async Task RequestDelegateFactory_AsParameters_SupportsRequiredMember() + { + // Arrange + static void TestAction([AsParameters] ParameterListRequiredStringFromDifferentSources args) { } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + // Act + await requestDelegate(httpContext); + + // Assert that the required modifier on members that + // are not nullable treats them as required. + Assert.Equal(400, httpContext.Response.StatusCode); + + var logs = TestSink.Writes.ToArray(); + + Assert.Equal(3, logs.Length); + + Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[0].EventId); + Assert.Equal(LogLevel.Debug, logs[0].LogLevel); + Assert.Equal(@"Required parameter ""string RequiredRouteParam"" was not provided from route.", logs[0].Message); + + Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[1].EventId); + Assert.Equal(LogLevel.Debug, logs[1].LogLevel); + Assert.Equal(@"Required parameter ""string RequiredQueryParam"" was not provided from query string.", logs[1].Message); + + Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[2].EventId); + Assert.Equal(LogLevel.Debug, logs[2].LogLevel); + Assert.Equal(@"Required parameter ""string RequiredHeaderParam"" was not provided from header.", logs[2].Message); + } + + private class ParameterListRequiredNullableStringFromDifferentSources + { + public HttpContext? HttpContext { get; set; } + + [FromRoute] + public required StringValues? RequiredRouteParam { get; set; } + + [FromQuery] + public required StringValues? RequiredQueryParam { get; set; } + + [FromHeader] + public required StringValues? RequiredHeaderParam { get; set; } + } + + [Fact] + public async Task RequestDelegateFactory_AsParameters_SupportsNullableRequiredMember() + { + // Arrange + static void TestAction([AsParameters] ParameterListRequiredNullableStringFromDifferentSources args) + { + args.HttpContext!.Items.Add("RequiredRouteParam", args.RequiredRouteParam); + args.HttpContext!.Items.Add("RequiredQueryParam", args.RequiredQueryParam); + args.HttpContext!.Items.Add("RequiredHeaderParam", args.RequiredHeaderParam); + } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + // Act + await requestDelegate(httpContext); + + // Assert that when properties are required but nullable + // we evaluate them as optional because required members + // must be initialized but they can be initialized to null + // when an NRT is required. + Assert.Equal(200, httpContext.Response.StatusCode); + + Assert.Null(httpContext.Items["RequiredRouteParam"]); + Assert.Null(httpContext.Items["RequiredQueryParam"]); + Assert.Null(httpContext.Items["RequiredHeaderParam"]); + } + +#nullable disable + private class ParameterListMixedRequiredStringsFromDifferentSources + { + public HttpContext HttpContext { get; set; } + + [FromRoute] + public required string RequiredRouteParam { get; set; } + + [FromRoute] + public string OptionalRouteParam { get; set; } + + [FromQuery] + public required string RequiredQueryParam { get; set; } + + [FromQuery] + public string OptionalQueryParam { get; set; } + + [FromHeader] + public required string RequiredHeaderParam { get; set; } + + [FromHeader] + public string OptionalHeaderParam { get; set; } + } + + [Fact] + public async Task RequestDelegateFactory_AsParameters_SupportsRequiredMember_NullabilityDisabled() + { + // Arange + static void TestAction([AsParameters] ParameterListMixedRequiredStringsFromDifferentSources args) { } + + var httpContext = CreateHttpContext(); + + var factoryResult = RequestDelegateFactory.Create(TestAction); + var requestDelegate = factoryResult.RequestDelegate; + + // Act + await requestDelegate(httpContext); + + // Assert that we only execute required parameter + // checks for members that have the required modifier + Assert.Equal(400, httpContext.Response.StatusCode); + + var logs = TestSink.Writes.ToArray(); + Assert.Equal(3, logs.Length); + + Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[0].EventId); + Assert.Equal(LogLevel.Debug, logs[0].LogLevel); + Assert.Equal(@"Required parameter ""string RequiredRouteParam"" was not provided from route.", logs[0].Message); + + Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[1].EventId); + Assert.Equal(LogLevel.Debug, logs[1].LogLevel); + Assert.Equal(@"Required parameter ""string RequiredQueryParam"" was not provided from query string.", logs[1].Message); + + Assert.Equal(new EventId(4, "RequiredParameterNotProvided"), logs[2].EventId); + Assert.Equal(LogLevel.Debug, logs[2].LogLevel); + Assert.Equal(@"Required parameter ""string RequiredHeaderParam"" was not provided from header.", logs[2].Message); + } +#nullable enable + private DefaultHttpContext CreateHttpContext() { var responseFeature = new TestHttpResponseFeature(); diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 76510c4d51f1..6f4d479e51ad 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -182,7 +182,9 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string // Determine the "requiredness" based on nullability, default value or if allowEmpty is set var nullabilityContext = new NullabilityInfoContext(); var nullability = nullabilityContext.Create(parameter); - var isOptional = parameter.HasDefaultValue || nullability.ReadState != NullabilityState.NotNull || allowEmpty; + var isOptional = parameter is PropertyAsParameterInfo argument + ? argument.IsOptional || allowEmpty + : parameter.HasDefaultValue || nullability.ReadState != NullabilityState.NotNull || allowEmpty; var parameterDescriptor = CreateParameterDescriptor(parameter, pattern); var routeInfo = CreateParameterRouteInfo(pattern, parameter, isOptional); diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 3db8a4b61105..b6aefb7330f5 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -527,6 +527,46 @@ static void AssertParameters(ApiDescription apiDescription, string capturedName } #nullable enable + public class AsParametersWithRequiredMembers + { + public required string RequiredStringMember { get; set; } + public required string? RequiredNullableStringMember { get; set; } + public string NonNullableStringMember { get; set; } = string.Empty; + public string? NullableStringMember { get; set; } + } + + [Fact] + public void SupportsRequiredMembersInAsParametersAttribute() + { + var apiDescription = GetApiDescription(([AsParameters] AsParametersWithRequiredMembers foo) => { }); + Assert.Equal(4, apiDescription.ParameterDescriptions.Count); + + Assert.Collection(apiDescription.ParameterDescriptions, + param => Assert.True(param.IsRequired), + param => Assert.False(param.IsRequired), + param => Assert.True(param.IsRequired), + param => Assert.False(param.IsRequired)); + } + +#nullable disable + public class AsParametersWithRequiredMembersObliviousContext + { + public required string RequiredStringMember { get; set; } + public string OptionalStringMember { get; set; } + } + + [Fact] + public void SupportsRequiredMembersInAsParametersObliviousContextAttribute() + { + var apiDescription = GetApiDescription(([AsParameters] AsParametersWithRequiredMembersObliviousContext foo) => { }); + Assert.Equal(2, apiDescription.ParameterDescriptions.Count); + + Assert.Collection(apiDescription.ParameterDescriptions, + param => Assert.True(param.IsRequired), + param => Assert.False(param.IsRequired)); + } +#nullable enable + [Fact] public void TestParameterIsRequired() { diff --git a/src/OpenApi/src/OpenApiGenerator.cs b/src/OpenApi/src/OpenApiGenerator.cs index 00df09773f77..f9039d6282ed 100644 --- a/src/OpenApi/src/OpenApiGenerator.cs +++ b/src/OpenApi/src/OpenApiGenerator.cs @@ -378,7 +378,9 @@ private List GetOpenApiParameters(MethodInfo methodInfo, Route } var nullabilityContext = new NullabilityInfoContext(); var nullability = nullabilityContext.Create(parameter); - var isOptional = parameter.HasDefaultValue || nullability.ReadState != NullabilityState.NotNull; + var isOptional = parameter is PropertyAsParameterInfo argument + ? argument.IsOptional + : parameter.HasDefaultValue || nullability.ReadState != NullabilityState.NotNull; var name = attributeName ?? (pattern.GetParameter(parameter.Name) is { } routeParameter ? routeParameter.Name : parameter.Name); var openApiParameter = new OpenApiParameter() { diff --git a/src/OpenApi/test/OpenApiGeneratorTests.cs b/src/OpenApi/test/OpenApiGeneratorTests.cs index 3438e7c2f155..d93e86ea7d2e 100644 --- a/src/OpenApi/test/OpenApiGeneratorTests.cs +++ b/src/OpenApi/test/OpenApiGeneratorTests.cs @@ -928,6 +928,46 @@ static void ValidateParameter(OpenApiOperation operation, string expectedName) ValidateParameter(GetOpenApiOperation(([FromHeader(Name = "headerName")] string param) => ""), "headerName"); } +#nullable enable + public class AsParametersWithRequiredMembers + { + public required string RequiredStringMember { get; set; } + public required string? RequiredNullableStringMember { get; set; } + public string NonNullableStringMember { get; set; } = string.Empty; + public string? NullableStringMember { get; set; } + } + + [Fact] + public void SupportsRequiredMembersInAsParametersAttribute() + { + var operation = GetOpenApiOperation(([AsParameters] AsParametersWithRequiredMembers foo) => { }); + Assert.Equal(4, operation.Parameters.Count); + + Assert.Collection(operation.Parameters, + param => Assert.True(param.Required), + param => Assert.False(param.Required), + param => Assert.True(param.Required), + param => Assert.False(param.Required)); + } +#nullable disable + + public class AsParametersWithRequiredMembersObliviousContext + { + public required string RequiredStringMember { get; set; } + public string OptionalStringMember { get; set; } + } + + [Fact] + public void SupportsRequiredMembersInAsParametersObliviousContextAttribute() + { + var operation = GetOpenApiOperation(([AsParameters] AsParametersWithRequiredMembersObliviousContext foo) => { }); + Assert.Equal(2, operation.Parameters.Count); + + Assert.Collection(operation.Parameters, + param => Assert.True(param.Required), + param => Assert.False(param.Required)); + } + private static OpenApiOperation GetOpenApiOperation( Delegate action, string pattern = null, diff --git a/src/Shared/PropertyAsParameterInfo.cs b/src/Shared/PropertyAsParameterInfo.cs index 04b6d379ec5b..9bb1ca006624 100644 --- a/src/Shared/PropertyAsParameterInfo.cs +++ b/src/Shared/PropertyAsParameterInfo.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using Microsoft.Extensions.Internal; @@ -182,7 +183,20 @@ public override bool IsDefined(Type attributeType, bool inherit) _underlyingProperty.IsDefined(attributeType, inherit); } - public new bool IsOptional => HasDefaultValue || NullabilityInfo.ReadState != NullabilityState.NotNull; + public new bool IsOptional => NullabilityInfo.ReadState switch + { + // Anything nullable is optional + NullabilityState.Nullable => true, + // In an oblivious context, the required modifier makes + // members non-optional + NullabilityState.Unknown => !_underlyingProperty.GetCustomAttributes().OfType().Any(), + // Non-nullable types are only optional if they have a default + // value + NullabilityState.NotNull => HasDefaultValue, + // Assume that types are optional by default so we + // don't greedily opt parameters into param checking + _ => true + }; public NullabilityInfo NullabilityInfo => _nullabilityInfo ??= _constructionParameterInfo is not null ?