Fix optional non-nullable struct query parameters with = default throwing in RequestDelegateFactory#66091
Fix optional non-nullable struct query parameters with = default throwing in RequestDelegateFactory#66091
= default throwing in RequestDelegateFactory#66091Conversation
…n RequestDelegateFactory Agent-Logs-Url: https://github.com/dotnet/aspnetcore/sessions/b06c1d39-20b4-4060-bca6-72f07cbf08ad Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
= default throwing in RequestDelegateFactory
…Parameters.cs Agent-Logs-Url: https://github.com/dotnet/aspnetcore/sessions/ae7b0e98-8ab8-436c-9727-b09547e4abba Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a runtime exception in minimal API binding when optional non-nullable struct query parameters use = default (reflection reports DefaultValue == null, which previously caused an invalid Expression.Constant(null, valueType)).
Changes:
- Update
RequestDelegateFactory.CreateDefaultValueExpressionto useExpression.Default(parameterType)whendefaultValueisnullfor non-nullable value types. - Add generator/runtime coverage for optional
[FromQuery] Guidand[FromQuery] DateTimeparameters declared with= default, with and without query values.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Http/Http.Extensions/src/RequestDelegateFactory.cs | Prevents ArgumentException by emitting a default(T) expression for non-nullable value types when reflection returns DefaultValue == null. |
| src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.QueryParameters.cs | Adds regression tests for optional struct query parameters using = default across runtime (factory) and compile-time (generator) paths. |
| [Theory] | ||
| [InlineData(null, "Id: 00000000-0000-0000-0000-000000000000")] | ||
| [InlineData("a0e1f2b3-c4d5-4e6f-7a8b-9c0d1e2f3a4b", "Id: a0e1f2b3-c4d5-4e6f-7a8b-9c0d1e2f3a4b")] | ||
| public async Task CanSetGuidParamAsOptionalWithDefaultValue(string queryValue, string expectedResponse) | ||
| { |
There was a problem hiding this comment.
queryValue is passed as null via [InlineData(null, ...)], but the parameter is declared as non-nullable string. Consider changing it to string? queryValue so the signature matches the test data and avoids nullable-analysis warnings.
| [Theory] | ||
| [InlineData(null, "Date: 0001-01-01T00:00:00.0000000")] | ||
| [InlineData("2024-01-15", "Date: 2024-01-15T00:00:00.0000000")] | ||
| public async Task CanSetDateTimeParamAsOptionalWithDefaultValue(string queryValue, string expectedResponse) |
There was a problem hiding this comment.
queryValue is provided as null in one [InlineData] case, but the parameter is declared string (non-nullable). Update it to string? queryValue to accurately model the test inputs and avoid nullable warnings.
| public async Task CanSetDateTimeParamAsOptionalWithDefaultValue(string queryValue, string expectedResponse) | |
| public async Task CanSetDateTimeParamAsOptionalWithDefaultValue(string? queryValue, string expectedResponse) |
Reflection reports
DefaultValue == nullfor non-nullable struct parameters with= default(e.g.,Guid,DateTime,TimeSpan).CreateDefaultValueExpressionwas passing thatnulldirectly toExpression.Constant(null, parameterType), which throwsArgumentException: Argument types do not matchfor non-nullable value types.Description
RequestDelegateFactory.cs: InCreateDefaultValueExpression, whendefaultValue is nullandparameterTypeis a non-nullable value type, useExpression.Default(parameterType)instead ofExpression.Constant(null, parameterType).RequestDelegateCreationTests.QueryParameters.cs: Added tests coveringGuidandDateTimeoptional parameters with= default, both with and without a query string value provided. Tests are placed inRequestDelegateCreationTests.QueryParameters.csso they run against bothRequestDelegateFactory(viaRuntimeCreationTests) andRequestDelegateGenerator(viaCompileTimeCreationTests).