Missing required value for constructor parameter

andrew-vdb commented 1 month ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

How difficult can this be

Checkbox in html form when its not true, simply not to submit the value in the property

 public record PostParameter([Required][EmailAddress] string Email, [Required][MinLength(8)] string Password, string? RememberMe = "unchecked");

 internal static async Task<Results<RedirectHttpResult, BadRequest>> Post(HttpContext httpContext, [FromForm] PostParameter parameter)

PostParameter has optional parameter but that optional parameter is not respected

BadHttpRequestException: Missing required value for constructor parameter 'RememberMe'.

Microsoft.AspNetCore.Http.RequestDelegateFactory+Log.FormDataMappingFailed(HttpContext httpContext, string parameterTypeName, string parameterName, FormDataMappingException exception, bool shouldThrow)

Expected Behavior

should simply accept (no exception) when client not sending "RememberMe" in the payload

Steps To Reproduce

No response

Exceptions (if any)

FormDataMappingException

.NET Version

8

Anything else?

No response

andrew-vdb commented 1 month ago

Workaround, dont use record

internal static async Task<Results<RedirectHttpResult, BadRequest>> Post(HttpContext httpContext, [FromForm][Required][EmailAddress] string Email, [FromForm][Required][MinLength(8)] string Password, [FromForm] string? RememberMe = "unchecked")

andrew-vdb commented 1 month ago

It should work without [FromForm] too It should just work....

andrew-vdb commented 1 month ago

The drama supporting "form" in minimal api https://github.com/dotnet/aspnetcore/issues/39430

captainsafia commented 4 weeks ago

@andrew-vdb Thanks for opening this issue! I hope I can clarify some of the confusion here.

There are currently two different form-binding strategies in minimal APIs:

A simplified form-binding strategy that has existed since .NET 7 that comes into play when you have [FromForm] on TryParsable types (like string, DateTime, etc.)
A complex form-binding strategy that was introduced in .NET 8 that comes into play when you have [FromForm] on a complex type (like your PostParameter record above). This form-binding strategy is shared with Blazor.

Workaround, dont use record

internal static async Task<Results<RedirectHttpResult, BadRequest>> Post(HttpContext httpContext, [FromForm][Required][EmailAddress] string Email, [FromForm][Required][MinLength(8)] string Password, [FromForm] string? RememberMe = "unchecked")

The reason the workaround that you describe here works is minimal APIs treats all parameters with a default value as optional. That same convention doesn't apply to the complex form-binding strategy shared with Blazor.

It looks like the reason this bug is happening is because the complex form-binding support always sets constructor parameters as required:

https://github.com/dotnet/aspnetcore/blob/0da8ea72b5434cbe8e1207d802f2270ca2f8ad4c/src/Components/Endpoints/src/FormMapping/Metadata/FormDataParameterMetadata.cs#L13

I'll see if I can update the complex form-binding implementation here so it handles constructor parameters in records with default values correctly (and nullable constructor parameters).

andrew-vdb commented 2 weeks ago

@captainsafia got bitten with different issue, basically with POCO object with minimal api, the default value in the property also not applied, this was simply work with controller

so the question is, is the workaround above, not using poco object is the way to go for minimal api? is this real recommendation? if true then can we have it somewhere it documentation?

dotnet / aspnetcore