Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for endpoint filters in minimal APIs #40491

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

captainsafia
Copy link
Contributor

@captainsafia captainsafia commented Mar 2, 2022

Resolves #37853.

This PR introduces an initial implementation of filters for route handlers in minimal APIs. As follow-ups to this work we want to be able to do the following:

  • Add support for a filter factory so that users can generate a filter based on the MethodInfo associated with the handler
  • Update the RouteHandlerFilterContext.Parameters definition to provide details about the parameter type, name, and (possibly) validity
  • Determine whether filters can run in scenarios where the handler typically wouldn't

/// This list is not read-only to premit modifying of existing parameters by filters.
/// </remarks>
/// </summary>
public IList<object?> Parameters { get; }
Copy link
Member

@davidfowl davidfowl Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public IList<object?> Parameters { get; }
public IReadOnlyList<object?> Parameters { get; }

/// <returns>The <see cref="RequestDelegateResult"/>.</returns>
#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
public static RequestDelegateResult Create(Delegate handler, RequestDelegateFactoryOptions? options = null)
public static RequestDelegateResult Create(Delegate handler, RequestDelegateFactoryOptions? options = null, IEnumerable<IRouteHandlerFilter>? filters = null)
Copy link
Member

@davidfowl davidfowl Mar 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make this part of the options instead? We need an API review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants