Skip to content
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

model binding multiple sources to a single class doesn't work as doumented #29295

Closed
KillerBoogie opened this issue May 17, 2023 · 8 comments
Closed
Labels
needs-more-info Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

@KillerBoogie
Copy link

KillerBoogie commented May 17, 2023

The first example under the section sources shows that you can mix various sources into one model. This does not work!

E.g.

[HttpPost]
public ActionResult Post(TestDetails testDetails)
{
    return Ok();
}

public record TestDetails
{
    [FromBody]
    public Artist? Artist { get; init; }

    [ModelBinder(typeof(EnvironmentBinder))]
    public IPAddress? IpAddress { get; init; }

    [FromHeader(Name = "Accept-Language")]
    public string? PreferredLanguages { get; init; }

    [FromQuery]
    public string? SelectedLanguage { get; init; }
}

public record Body
{
    public string? Name { get; init; }

    public string? StageName { get; init; }
}

Calling the endpoint creates an error stating that Artist must not be empty.

To make it work either the following option must be set

builder.Services.AddMvc().ConfigureApiBehaviorOptions(options => {
    options.SuppressInferBindingSourcesForParameters = true;
});

or (from .Net 6 on?) the method parameter must be declared as [FromQuery]:

[HttpPost]
public ActionResult Post([FromQuery] TestDetails testDetails)
{
    return Ok();
}

This is totally missing in the documentation and cost me several days to figure out.


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

@dotnet-bot dotnet-bot added ⌚ Not Triaged Source - Docs.ms Docs Customer feedback via GitHub Issue labels May 17, 2023
@Rick-Anderson
Copy link
Contributor

Rick-Anderson commented May 17, 2023

None of the code you posted is in this tutorial.

Please reproduce a problem with the code in this tutorial.

From the article:

The following controller action uses the DateRange class to bind a date range:

// GET /WeatherForecast/ByRange?range=7/24/2022,07/26/2022
public IActionResult ByRange([FromQuery] DateRange range)
{
    if (!ModelState.IsValid)
        return View("Error", ModelState.Values.SelectMany(v => v.Errors));

    var weatherForecasts = Enumerable
        .Range(1, 5).Select(index => new WeatherForecast
        {
            Date = DateTime.Now.AddDays(index),
            TemperatureC = Random.Shared.Next(-20, 55),
            Summary = Summaries[Random.Shared.Next(Summaries.Length)]
        })
        .Where(wf => DateOnly.FromDateTime(wf.Date) >= range.From
                     && DateOnly.FromDateTime(wf.Date) <= range.To)
        .Select(wf => new WeatherForecastViewModel
        {
            Date = wf.Date.ToString("d"),
            TemperatureC = wf.TemperatureC,
            TemperatureF = 32 + (int)(wf.TemperatureC / 0.5556),
            Summary = wf.Summary
        });

    return View("Index", weatherForecasts);
}

@KillerBoogie
Copy link
Author

KillerBoogie commented May 17, 2023

The code I posted is an extended example to show the problem.

In the article at section Sources the example shows:

public class Instructor
{
    public int Id { get; set; }

    [FromQuery(Name = "Note")]
    public string? NoteFromQueryString { get; set; }

    // ...
}

When using the class in a controller method like this...

[HttpPost]
public ActionResult PostOneClass(Instructor instructor)
{
    return Ok(instructor);
}

...the property NoteFromQueryString is not set. It is always null. So the example does not work!

When the instructor class is attributed with a [FromQuery] ...

[HttpPost]
public ActionResult PostOneClass([FromQuery] Instructor instructor)
{
   return Ok(instructor);
}

...the NoteFromQueryString property is set, but not the Id.

When the Id is marked additionally as [FromBody] ...

public class Instructor
{
    [FromBody]
    public int Id { get; set; }

    [FromQuery(Name = "Note")]
    public string? NoteFromQueryString { get; set; }

}

...a 400 error is returned, stating: "The JSON value could not be converted to System.Int32..."

Keeping [FromBody] in the Instructor class and removing [FromQuery] from the controller method makes NoteFromQueryString return null again.

The only solution I found is to put the body parameter(s) in a separate class...

public class Instructor
{
    [FromBody]
    public Body body { get; set; }

    [FromQuery(Name = "Note")]
    public string? NoteFromQueryString { get; set; }

}
public class Body
{
    public int Id { get; set; }
   //...
}

... and use [FromQuery] in the method:

[HttpPost]
public ActionResult PostOneClass([FromQuery] Instructor instructor)
{
    return Ok(instructor);
}

Without [FromQuery] in the method a request returns a 400 error, stating: "The body field is required."

So, the example doesn't work and the only working solution is not presented.

Additionally, the example from the article is not displayed correctly in Swagger UI. The query parameter is displayed as part of the body!

@Rick-Anderson
Copy link
Contributor

@serpent5 @sammychinedu2ky @fiyazbinhasan please review

@KillerBoogie
Copy link
Author

KillerBoogie commented May 18, 2023

Yes, this is correctly stated. But, even if '[FromBody]' is not stated as in the example above it doesn't work either, because it is added as a convention. I just listed all options and outcomes in my post.

My main point is that there is no way of getting the stated example to work with the information given in the article. The critical information to make this example work is not obvious and no where found in the article. It is only stated that it won't work with `[FromBody]', but not how to make it work. How do you make this example from the sources section above work? Please tell me and then state it in the article.

public class Instructor
{
    public int Id { get; set; }

    [FromQuery(Name = "Note")]
    public string? NoteFromQueryString { get; set; }

    // ...
}

I even didn't know that it is possible before I saw the example and then failed when trying it out. I lost a week of my time just to figure out how it works by testing all options instead of reading a good documentation for 10 mins.

Here is another workaround, creating a custom [RequestAttribute]: https://josef.codes/model-bind-multiple-sources-to-a-single-class-in-asp-net-core/

@sammychinedu2ky
Copy link
Contributor

Yes, this is correctly stated. But, even if '[FromBody]' is not stated as in the example above it doesn't work either, because it is added as a convention. I just listed all options and outcomes in my post.

My main point is that there is no way of getting the stated example to work with the information given in the article. The critical information to make this example work is not obvious and no where found in the article. It is only stated that it won't work with `[FromBody]', but not how to make it work. How do you make this example from the sources section above work? Please tell me and then state it in the article.

public class Instructor
{
    public int Id { get; set; }

    [FromQuery(Name = "Note")]
    public string? NoteFromQueryString { get; set; }

    // ...
}

I even didn't know that it is possible before I saw the example and then failed when trying it out. I lost a week of my time just to figure out how it works by testing all options instead of reading a good documentation for 10 mins.

Here is another workaround, creating a custom [RequestAttribute]: https://josef.codes/model-bind-multiple-sources-to-a-single-class-in-asp-net-core/

ooh yh you're right

@sammychinedu2ky
Copy link
Contributor

@KillerBoogie

Copy link

This issue has been automatically closed due to no response from the original author. Feel free to reopen it if you have more information that can help us investigate the issue further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-more-info Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
None yet
Development

No branches or pull requests

4 participants