Introduce `IParameterBindingMetadata` to remove unbounded reflection in EndpointMetadataApiDescriptionProvider

[captainsafia]

Background and Motivation

The EndpointMetadataApiDescriptionProvider is a an implementation of IApiDescriptionProvider that is used to generate ApiDescription instances for minimal APIs that can be used by consumers of ApiExplorer (like Microsoft.AspNetCore.OpenApi) when generating descriptions of the API implementation.

EndpointMetadataApiDescriptionProvider currently relies on unbounded reflection in all scenarios (whether using minimal APIs with static code gen via RequestDelegateGenerator or dynamic code gen via RequestDelegateFactory) to resolve the following information:

• Whether or not a given parameter has a TryParse method or implements IParsable
• Whether or not a given parameter is an implementation of type that exposes BindAsync
• The exploded set of parameters that have been aggregated with the [AsParameters] attribute

It resolves this information using the existing APIs exposed in the ParameterBindingCache. The use of unbounded reflection in this class prevents us from making the entire OpenAPI pipeline trim friendly. To resolve this issue, we will introduce a new metadata type (IParameterBindingMetadata) that will be inserted into endpoints by either RDG or RDF when the endpoints are compiled and used.

Proposed API

// Assembly: Microsoft.AspNetCore.Http.Abstractions

namespace Microsoft.AspNetCore.Http.Metadata;

+ public interface IParameterBindingMetadata
+ {
+   string ParameterName { get; }
+   bool IsTryParsable { get; }
+   bool IsBindAsync { get; }
+   IEnumerable<(ParameterInfo, bool)>? AsParameters { get; }
+ }

public interface IProducesResponseTypeMetadata
{
+   bool IsInferredAwaitable { get; set; }
}

Usage Examples

var parameterBindingMetadata = routeEndpoint.Metadata
    .GetOrderedMetadata<IParameterBindingMetadata>()
    .SingleOrDefault(m => m.ParameterName == parameter.Name);
if (parameterBindingMetadata?.HasTryParse == true)
{
    return BindingSource.Query;
}

Alternative Designs

• Instead of exposing a single IParameterBindingMetadata instance for each parameter, we could instead expose a single metadata item that allows lookup into a collection of instances using the parameter name as a key.

Risks

• The shape of this API strongly conforms to the scenarios where we currently use the ParameterBindingCache for method resolution. If the parameter binding implementation for minimal APIs changes in the future, the shape of this API will have to be modified.

[amcasey]

[API Review]

• Do the AsParameters get their own IParameterBindingMetadata?
  • Yes
• Do we need a full ParameterInfo or can we provide a subset of that info?
• Or, could it be IEnumerable<IParameterBindingMetadata> (to which we might need to add IsOptional)?
• If we change "Is" to "Has" it might read a little better
• AsProperties -> PropertiesAsParameters
• IsInferredAwaitable will default to false
  • Should throw by default
  • Doesn't need to be settable (probably)
  • Rename to IsAwaitable

[captainsafia]

Making updates to the proposal in response to the feedback that I received and some additional work in the area:

// Assembly: Microsoft.AspNetCore.Http.Abstractions

namespace Microsoft.AspNetCore.Http.Metadata;

+ public interface IParameterBindingMetadata
+ {
+   string Name { get; }
+   bool HasTryParse { get; }
+   bool HasBindAsync { get; }
+   bool IsOptional { get; }
+   ParameterInfo ParameterInfo { get; }
+ }

public interface IProducesResponseTypeMetadata
{
+   bool IsAwaitable { get; }
}

One open question from review was:

Do we need a full ParameterInfo or can we provide a subset of that info?

Although we only look at certain parts of the ParameterInfo (custom attributes to figure out the binding source, parameter name, type, etc.), ApiExplorer still requires that we encapsulate the ParameterInfo into the EndpointParameterDescriptor object so we can't get away with totally removing references to it (ref).

Or, could it be IEnumerable (to which we might need to add IsOptional)?

I opted to not make the metadata contain an IEnumerable mostly to make unwrapping the collection a little bit easier.

var bindingMetadata = endpoint.Metadata.GetOrderedMetadata<IParameterBindingMetadata>();
// vs
var bindingMetadata = endpoint.Metadata.GetOrderedMetadata<SomeContainerType>().BindingMetadata;

[amcasey]

[API Review]

• Was the AsParameter member necessary for source generation
  • The PR works without it, so apparently not
  • https://github.com/dotnet/aspnetcore/commit/5c1bcd733dd316a472a04daaebed62ff062a2bb7
• Are we doing anything similar for MVC actions?
  • No, since it doesn't presently support AOT
• Do string parameters have HasTryParse true?
  • Yes
• IsAwaitable looks really weird on IProducesResponseTypeMetadata
  • true indicates that Type is an unwrapped type
  • Why would you need to know whether the result is wrapped in Task?
    • Might matter for return type, seems unnecessary otherwise
    • OpenAPI only cares about the unwrapped type
  • Determinable from reflection info (non-trivially, because of custom awaitables)
    • But that's not AOT-friendly
    • We're already limiting to Task and ValueTask in RDG (but not in RDF)
• Do we need a DIM?
  • Presumably, the compiler will tell us

[amcasey]

Seems promising, but let's try to get rid of IProducesResponseTypeMetadata.IsAwaitable.

IParameterBindingMetadata changes approved!