Update Endpoint metadata providers to include EndpointBuilder param

[halter73]

Background and Motivation

With #43000, we've approved adding RequestDelegateFactoryOptions.EndpointBuilder and EndpointFilterFactoryContext.EndpointBuilder so filters can get full access to more metadata like the RoutePattern (with a downcast) and DisplayName.

I think IEndpointMetadataProvider and IEndpointParameterMetadataProvider deserve the same treatment.

Also see #42827 (comment) for more context.

Proposed API

namespace Microsoft.AspNetCore.Http.Metadata;


public interface IEndpointMetadataProvider
{
-     static abstract void PopulateMetadata(EndpointMetadataContext context);
+     static abstract void PopulateMetadata(MethodInfo method, EndpointBuilder builder);
}

public interface IEndpointParameterMetadataProvider
{
-     static abstract void PopulateMetadata(EndpointParameterMetadataContext parameterContext);
+     static abstract void PopulateMetadata(ParameterInfo parameter, EndpointBuilder builder);
}

- public sealed class EndpointMetadataContext
- {
-     public EndpointMetadataContext(MethodInfo method, IList<object> endpointMetadata, IServiceProvider applicationServices);
-     public MethodInfo Method { get; }
-     public IList<object> EndpointMetadata { get; }
-     public IServiceProvider ApplicationServices { get; }
- }

- public sealed class EndpointParameterMetadataContext
- {
-     public EndpointParameterMetadataContext(ParameterInfo parameter, IList<object> endpointMetadata, IServiceProvider applicationServices);
-     public ParameterInfo Parameter { get; }
-     public IList<object> EndpointMetadata { get; }
-     public IServiceProvider ApplicationServices { get; }
- }

Usage Examples

static void IEndpointMetadataProvider.PopulateMetadata(MethodInfo method, EndpointBuilder builder)
{
    builder.Metadata.Add(new ProducesResponseTypeMetadata(typeof(TValue), StatusCodes.Status200OK, "application/json"));
}

Alternative Designs

We could change context. to reference a builder in case we want to add more parameters in the future that are not on the EndpointBuilder in some way. I find this is clunky though, and I'm not sure how likely additional future parameters really are.

namespace Microsoft.AspNetCore.Http.Metadata;

public sealed class EndpointMetadataContext
{
-     public EndpointMetadataContext(MethodInfo method, IList<object> endpointMetadata, IServiceProvider applicationServices);
+     public EndpointMetadataContext(MethodInfo method, EndpointBuilder builder);

      public MethodInfo Method { get; }
-     public IList<object> EndpointMetadata { get; }
-     public IServiceProvider ApplicationServices { get; }
+     public EndpointBuilder EndpointBuilder { get; }
}

public sealed class EndpointParameterMetadataContext
{
-     public EndpointParameterMetadataContext(ParameterInfo parameter, IList<object> endpointMetadata, IServiceProvider applicationServices);
-     public EndpointParameterMetadataContext(ParameterInfo prameter, EndpointBuilder builder);

       public ParameterInfo Parameter { get; }
-     public IList<object> EndpointMetadata { get; }
-     public IServiceProvider ApplicationServices { get; }
+     public EndpointBuilder EndpointBuilder { get; }
}

Risks

With the primary proposal, we lose out on having a context, so it's harder to add parameters in the future if we find it will be necesarry.

Third-party IEndpointMetadataProvider implementations need to rewrite their PopulateMetadata methods with either change, but it should be pretty mechanical.

@DamianEdwards

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

• Will this work with MVC?
  • Mvc's EndpointMetadataConvention.cs doesn't have access to one. It's not clear we could flow one through.
  • We could fake one, but that seems bad if metadata is the only thing that's supported.
• Without evidence this can work well in MVC too, we reject this API.
• We still like Add APIs to make it possible for users of RouteEndpointBuilder to use filters #42592 which doesn't have these problems.

API Rejected! (unless we find an easy way to do this in MVC)

[halter73]

Okay. I think this is workable. But we also need to add a new API to RDF to allow IEndpointMetadataProvider IEndpointParameterMetadataProvider metadata to be added before we run any route-specific conventions. This requires making RDF two-phase so metadata can be inferred before all the filters run because filters can be added by route-specific conventions.

namespace Microsoft.AspNetCore.Http.Metadata;

public interface IEndpointMetadataProvider
{
-     static abstract void PopulateMetadata(EndpointMetadataContext context);
+     static abstract void PopulateMetadata(MethodInfo method, EndpointBuilder builder);
}

public interface IEndpointParameterMetadataProvider
{
-     static abstract void PopulateMetadata(EndpointParameterMetadataContext parameterContext);
+     static abstract void PopulateMetadata(ParameterInfo parameter, EndpointBuilder builder);
}

- public sealed class EndpointMetadataContext
- {
-     public EndpointMetadataContext(MethodInfo method, IList<object> endpointMetadata, IServiceProvider applicationServices);
-     public MethodInfo Method { get; }
-     public IList<object> EndpointMetadata { get; }
-     public IServiceProvider ApplicationServices { get; }
- }

- public sealed class EndpointParameterMetadataContext
- {
-     public EndpointParameterMetadataContext(ParameterInfo parameter, IList<object> endpointMetadata, IServiceProvider applicationServices);
-     public ParameterInfo Parameter { get; }
-     public IList<object> EndpointMetadata { get; }
-     public IServiceProvider ApplicationServices { get; }
- }

namespace Microsoft.AspNetCore.Http;

+ public sealed class RequestDelegateMetadataResult
+ {
+     public required IReadOnlyList<object> EndpointMetadata { get; init; }
+ }

public static class RequestDelegateFactory
{
+     public static RequestDelegateMetadataResult InferMetadata(MethodInfo methodInfo, RequestDelegateFactoryOptions? options = null);
}

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

1. The current proposal with EndpointBuilders passed to the metadata providers is dependent on the RequestDelegateFactoryOptions in Expose entire EndpointBuilder to filter factories #43000.
2. There seems to be general consensus that it makes sense to make RDF two phase so route-specific conventions can observe and override inferred metadata.
   • There is renewed debate in Expose entire EndpointBuilder to filter factories #43000 however.
   • We could still make RDF two phase while only exposing the IList<object> on a context like before

On a personal note, I would much prefer to use EndpointBuilder. If we make RDF two phase, we can run the filter factory in the inference phase and make it visible to route-specific conventions just like these metadata providers.