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 https://github.com/dotnet/aspnetcore/pull/42827#discussion_r934757604 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

[ghost]

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 #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);
}

[ghost]

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 #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 #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.

[captainsafia]

Cross-refing https://github.com/dotnet/aspnetcore/issues/43330 as a scenario for this API.

[halter73]

Description of the order metadata is added.

Order of Metadata addition today

1. MethodInfo (RouteEndpointDataSource)
2. HttpMethodMetadata (RouteEndpointDataSource)
3. Group conventions (outer to inner) (RouteEndpointDataSource)
4. Attributes (RouteEndpointDataSource)
5. Route-specific conventions (RouteEndpointDataSource)
6. Implicitly inferred metadata (RDF.Create)
   1. AcceptsMetadata (inserted at beginning)
7. Static interface metadata providers (RDF.Create)
8. Endpoint Filters (RDF.Create)
9. Route-specific finally conventions (RouteEndpointDataSource)
10. Group finally conventions (inner to outer) (RouteEndpointDataSource)

Order of Metadata addition tomorrow

1. MethodInfo (RouteEndpointDataSource)
2. HttpMethodMetadata (RouteEndpointDataSource)
3. Implicitly inferred metadata (RDF.InferMetadata)
   1. AcceptsMetadata
   2. ProducesMetadata
4. Static interface metadata providers (RDF.InferMetadata)
5. Group conventions (outer to inner) (RouteEndpointDataSource)
6. Attributes (RouteEndpointDataSource)
7. Route-specific conventions (RouteEndpointDataSource)
8. ~Endpoint Filters mutate metadata (RDF.Create)~
9. Route-specific finally conventions (RouteEndpointDataSource)
10. Group finally conventions (inner to outer) (RouteEndpointDataSource)

There is a subtle behavior where metadata is not re-added only if the options is reused between InferMetadata() and Create().

• Can we make InferMetadata void?
  • Actually, let's flow the result to create to make it more explicit that we've already done inference.

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 RequestDelegateMetadataResult();
+     public required IReadOnlyList<object> EndpointMetadata { get; init; }
+ }

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

-     public static RequestDelegateResult Create(Delegate handler, RequestDelegateFactoryOptions? options = null);
+     public static RequestDelegateResult Create(Delegate handler, RequestDelegateFactoryOptions? options);
+     public static RequestDelegateResult Create(Delegate handler, RequestDelegateFactoryOptions? options = null, RequestDelegateMetadataResult? metadataResult = null);

-     public static RequestDelegateResult Create(MethodInfo methodInfo, Func<HttpContext, object>? targetFactory = null, RequestDelegateFactoryOptions? options = null)
+     public static RequestDelegateResult Create(MethodInfo methodInfo, Func<HttpContext, object>? targetFactory, RequestDelegateFactoryOptions? options);
+     public static RequestDelegateResult Create(MethodInfo methodInfo, Func<HttpContext, object>? targetFactory = null, RequestDelegateFactoryOptions? options = null, RequestDelegateMetadataResult? metadataResult = null);
}

API Approved!

[ghost]

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.

[DamianEdwards]

@halter73 seems this should this be moved to rc.2?