Added support for arrays of types with TryParse, StringValues and string[] for query strings and headers

[davidfowl]

• Added inferred query support for methods where the body inference is disabled.
• Added a tests.

Fixes #32516
Fixes #36726

[martincostello]

Two questions about how supporting this could work when finished.

1. Say for example that a query string contained values like 1,2,,4 because there's a blank/missing entry. Would it be explicitly ignored because empty (so you'd get new int[] { 1, 2, 4 }), would it fail the binding because the int.TryParse() call would return false, or could the user opt into one behaviour vs. the other?
2. Would this support query strings in the formats a=1,2,3, a=1&a=2&a=3, or both?

[davidfowl]

Say for example that a query string contained values like 1,2,,4 because there's a blank/missing entry. Would it be explicitly ignored because empty (so you'd get new int[] { 1, 2, 4 }), would it fail the binding because the int.TryParse() call would return false, or could the user opt into one behaviour vs. the other?

This is a good question, currently I think it would fail to bind but I could go either way here (ignoring empty entries).

Would this support query strings in the formats a=1,2,3, a=1&a=2&a=3, or both?

No, it will only support the second, not the first. The one thing I don't want to support multiple formats unless they are common. MVC supports LOTS of formats https://docs.microsoft.com/en-us/aspnet/core/mvc/models/model-binding?view=aspnetcore-6.0#collections, that won't be supported here. I chose the most obvious one (the latter in your above sample).

[martincostello]

No, it will only support the second, not the first.

That's fair - I figured I'd ask the question about how much of MVC's leniency would come through as I've seen both approaches in the wild.

[davidfowl]

That's fair - I figured I'd ask the question about how much of MVC's leniency would come through as I've seen both approaches in the wild.

If this is common, we can support it. I'll let others chime in of course 😄. I'm inclined to support a subset of whatever MVC supports and it doesn't support slapping multiple values into a single query string value.

using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddControllers();
var app = builder.Build();

app.MapControllers();

app.Run();

public class MyController : ControllerBase
{
    [HttpGet("/")]
    public string Get(int[] values) => string.Join("&", values);
}

Sending this /?values=1,2,3 doesn't print anything.

This is because the collection model binder uses the simple model binder (type converter) to convert values from strings to the target type. This will try to parse the "1,2,3" as an int as it doesn't split things first. Thats probably good because you'd also need to decide what happens when you do; ?a=1,2,3&a=4

[davidfowl]

More copied logic between Map and the api description provider 😢

[halter73]

Can we move this to shared source to prevent these from going out of sync? Maybe rename ParameterBindingMethodCache to be more general. If not, I'd say just add a comment to the ShouldDisableInferredBody(string method) in EndpointRouteBuilderExtensions to keep these in sync.

[halter73]

Oh, I just noticed that this should be called against all the methods in IHttpMethodMetadata.HttpMethods, not just the one we're creating the ApiDescription for currently to align with the logic in EndpointRouteBuilderExtensions.

[davidfowl]

OK this is done and ready for review.