Skip to content

Missing required value for constructor parameter #56234

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

Open
1 task done
andrew-vdb opened this issue Jun 14, 2024 · 5 comments
Open
1 task done

Missing required value for constructor parameter #56234

andrew-vdb opened this issue Jun 14, 2024 · 5 comments
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc

Comments

@andrew-vdb
Copy link

Is there an existing issue for this?

  • 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

@ghost ghost added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 14, 2024
@andrew-vdb
Copy link
Author

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
Copy link
Author

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

@andrew-vdb
Copy link
Author

The drama supporting "form" in minimal api
#39430

@captainsafia
Copy link
Member

@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:

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
Copy link
Author

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Projects
None yet
Development

No branches or pull requests

2 participants