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

Merged
merged 5 commits into from
Mar 4, 2022

Conversation

captainsafia
Copy link
Member

@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

@captainsafia captainsafia requested a review from a team March 2, 2022 05:01
@ghost ghost added the area-runtime label Mar 2, 2022
@captainsafia captainsafia added area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-runtime labels Mar 2, 2022
@captainsafia captainsafia marked this pull request as ready for review March 2, 2022 19:09
@captainsafia captainsafia requested review from a team and removed request for Tratcher and javiercn March 2, 2022 19:09
@captainsafia captainsafia enabled auto-merge (squash) March 3, 2022 18:38
@captainsafia
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

/// <summary>
/// A list of parameters provided in the current request to the filter.
/// <remarks>
/// This list is not read-only to premit modifying of existing parameters by filters.
Copy link
Member

Choose a reason for hiding this comment

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

Next PR:

Suggested change
/// This list is not read-only to premit modifying of existing parameters by filters.
/// This list is not read-only to permit modifying of existing parameters by filters.

/// <param name="builder">The <see cref="RouteHandlerBuilder"/>.</param>
/// <param name="filter">The <see cref="IRouteHandlerFilter"/> to register.</param>
/// <returns>A <see cref="RouteHandlerBuilder"/> that can be used to further customize the route handler.</returns>
public static RouteHandlerBuilder AddFilter(this RouteHandlerBuilder builder, IRouteHandlerFilter filter)
Copy link
Member

Choose a reason for hiding this comment

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

Don't see any tests using these methods, should add that in the next PR 😃

@captainsafia captainsafia merged commit 7e05088 into dotnet:main Mar 4, 2022
@ghost ghost added this to the 7.0-preview3 milestone Mar 4, 2022

namespace Microsoft.AspNetCore.Http;

internal class DelegateRouteHandlerFilter : IRouteHandlerFilter
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to be sealed

/// <param name="builder">The <see cref="RouteHandlerBuilder"/>.</param>
/// <param name="filter">The <see cref="IRouteHandlerFilter"/> to register.</param>
/// <returns>A <see cref="RouteHandlerBuilder"/> that can be used to further customize the route handler.</returns>
public static RouteHandlerBuilder AddFilter(this RouteHandlerBuilder builder, IRouteHandlerFilter filter)
Copy link
Contributor

@WeihanLi WeihanLi Mar 4, 2022

Choose a reason for hiding this comment

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

WithFilter may be better I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for endpoint filters
7 participants