Mvc: Controller parameter inference and complex types for query strings

[kakins]

Is this a Bug or Feature request?:

Feature request

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

Decorate a controller with [ApiController]
Create a controller method with a complex object as a parameter, decorated with [FromQuery].
Calling the controller method will return an HTTP 400.

Description of the problem:

I believe using parameter inference causes a 400 response when passing in a complex object as a query string parameter.

I assume this is because complex objects are inferred as coming from the body, as noted here: https://docs.microsoft.com/en-us/aspnet/core/web-api/index?view=aspnetcore-2.1

However, there are times when I both want inference in a controller but also be able to override inference on a specific action method. For instance when passing in a complex object for a query string:

        [HttpGet("")]
        public async Task<IActionResult> Get([FromQuery] PositionFilterViewModel filter)
        {
            PagedViewModel<PositionViewModel> positions = await _domain.GetPositionsAsync(filter);

            return Ok(positions);
        }

Unless I'm misunderstanding something, even when I specify [FromQuery] here, I still get an http 400 response because I assume inference is expecting the object in the body. To me using a complex object as a query string seems like a common scenario.

It'd be nice to not have to go "all or nothing", forcing me to either have the controller decorated with [ApiController] or leave it out altogether. I may perhaps want to add another controller method where inference is enforced.

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:

Microsoft.AspNetCore.Mvc 2.1.1

[khellang]

You're most likely getting a 400 error because the model is failing validation. API controllers will automatically check ModelState.IsValid and return back 400 if there are errors.

Can you show the code for PositionFilterViewModel?

Do you have any non-nullable propertied that you're omitting?

[khellang]

If you want to disable the ModelStateInvalidFilter, you can set the SuppressModelStateInvalidFilter option in your Startup:

services.Configure<ApiBehaviorOptions>(o => o.SuppressModelStateInvalidFilter = true);

[kakins]

@khellang Thanks for the response. I do not necessarily want to suppress the model state validation. Here is my view model:

    public class PositionFilterViewModel : PagedViewModel
    {
        public List<string> Workstation { get; set; } = new List<string>();
        public List<string> Adloc { get; set; } = new List<string>();
        public List<string> Pin { get; set; } = new List<string>();
        public List<string> JobProfile { get; set; } = new List<string>();
        public List<string> Worker { get; set; } = new List<string>();
        public bool? IsVacant { get; set; }
        public bool? HasPrepAllocations { get; set; }
        public bool? HasErrorsOrWarnings { get; set; }
    }

[khellang]

And PagedViewModel?

[kakins]

Here is PagedViewModel. I believe all properties on this class are set when making the call to the controller.

    public class PagedViewModel
    {
        public int CurrentPageNumber { get; set; }
        public int ItemsPerPage { get; set; }
    }

[mkArtakMSFT]

@dougbu, isn't this an effect of https://github.com/aspnet/Mvc/issues/7712 ?
Though this is not a top-level collection scenario.

[dougbu]

@mkArtakMSFT this is not directly related to #7712.

@kakins you're combining two Mvc features that are not intended to work together. [ApiController] adds new conventions specific to API controllers while your controller is a view controller.

That said, you seem to appreciate the ModelStateInvalidFilter. Suggest adding that instead of trying to use [ApiController] for views e.g.
``` c#
[TypeFilter(typeof(ModelStateInvalidFilter))]
public class YourController : Controller
{
[HttpGet("")]
public async Task Get([FromQuery] PositionFilterViewModel filter)
{
PagedViewModel positions = await _domain.GetPositionsAsync(filter);

    return Ok(positions);
}

}
```

---

By the way, binding a complex type from the query string is entirely supported. I suspect [ApiController] causes model state errors in this particular case because values are submitted with names like filter.Workstation. [ApiController] doesn't use the parameter name unless your code is explicit e.g. public async Task<IActionResult> Get([FromQuery(Name="filter")] PositionFilterViewModel filter). But, again, use the filter rather than opting out of the many conventions that are not intended for your scenario.

[kakins]

@dougbu I guess I'm a _little_ confused on what you mean by a view controller? My controller is indeed supposed to be an API controller, no views are being returned -- and by that I mean nowhere am I returning View(). I also inherit from ControllerBase per the documentation for the new API controller conventions.

You may be inferring that I am using a view controller by my use of a "view model"? If so, know that this controller is called from an Angular front end. The "view model" is just the shape of the JSON data I wish to return to the caller.

So with that said, I'm still not quite sure I understand what you meant by values submitted with names like filter.Workstation. If you mean the values passed in from the query string in the URL, I'm fairly certain they look like the following: http://myurl.com/api/controller?workstation=A&workstation=B

[khellang]

Are you always including CurrentPageNumber and ItemsPerPage in your query strings? If not, I'd make them nullable.

[dougbu]

@kakins you're correct I was confused by your "view model" types.

What you're doing should Just Work:tm: because [FromQuery] will override the BindingSource.Body inference. As @khellang suggests, http://myurl.com/api/controller?currentPageNumber=3&itemsPerPage=15 should silence the most likely model binding errors (int properties are implicitly required).

If other errors occur, please provide us with a minimal repro project that illustrates the problem, preferably hosted in a GitHub repo.

[dougbu]

@kakins thank-you for your feedback. We're closing this issue as the questions asked here have been answered.