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

Allow to define HTTP filters with custom methods similar to controller definition #8179

Closed
dstepanov opened this issue Oct 17, 2022 · 11 comments
Assignees
Labels
status: next major version The issue will be considered for the next major version
Milestone

Comments

@dstepanov
Copy link
Contributor

Feature description

It would be nice to remove the requirement to always write reactive HTTP filters in Micronaut 4.

The requirements would be the ability to write a filter method using imperative, reactive (reactive-streams), Java 15 Flow, Kotlin coroutines, and completable future, just like it's possible to define a controller method.

The proposal to introduce two annotations:

  1. @RequestFilter - method can accept a HttpRequest (possible mutable depending on Server/Client), and it can mutate its state or return a different instance. The method can return reactive type.

Alternative to writing:

    @Override
    public Publisher<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
        // Mutate request
        return chain.proceed(request);
    }

    @Override
    public Publisher<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
        return Mono.from(fetcher).flatMapMany(result -> {
            // Mutate request in a reactive way
            return chain.proceed(request);
        });
    }
  1. @ResponseFilter- method can accept an originating request, a HttpResponse (possible mutable depending on Server/Client), and it can mutate its state or return a different instance. The method can return reactive type.

Alternative to writing:

    @Override
    public Publisher<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
        return Mono.from(chain.proceed(request)).map(response -> {
                // Mutate response
            return response;
        });
    }

    @Override
    public Publisher<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
        return Mono.from(chain.proceed(request)).flatMap(response -> {
            return Mono.from(fetcher).map(val -> {
                // Mutate response in a reactive way
                return response;
            });
        });
    }

Examples:

@Filter(Filter.MATCH_ALL_PATTERN)
public class MyFilter {

    MyExternalService service = null;

    @Order(Ordered.LOWEST_PRECEDENCE) // Log after all filters modify the request
    @RequestFilter
    public void loggingRequestFilter(HttpRequest<?> request) {
        System.out.println("REQUEST: " + request);
    }

    @RequestFilter
    public void loggingOnlyGets(HttpRequest<?> request) {
        System.out.println("REQUEST: " + request);
    }

    @RequestFilter
    public HttpRequest<?> modifyingRequestFilter(HttpRequest<?> request) {
        MutableHttpRequest<?> mutableHttpRequest = request.mutate();
        mutableHttpRequest.getHeaders().add("xyz", "abc");
        return mutableHttpRequest;
    }

    @RequestFilter
    public Publisher<HttpRequest<?>> modifyingReactiveRequestFilter(HttpRequest<?> request) {
        return Mono.from(service.fetchSomeReactive()).map(val -> {
            MutableHttpRequest<?> mutableHttpRequest = request.mutate();
            mutableHttpRequest.getHeaders().add("xyz", val);
            return mutableHttpRequest;
        });
    }

    @Order(Ordered.HIGHEST_PRECEDENCE) // Log right after retrieved
    @ResponseFilter
    public void simpleResponseFilter(HttpResponse<?> response) {
        System.out.println("RESPONSE: " + response);
    }

    @ResponseFilter
    public void simpleResponseFilter(MutableHttpResponse<?> response) {
        response.header("abc", "xyz");
    }

    @ResponseFilter
    public void simpleResponseFilterWithRequest(HttpRequest<?> request, MutableHttpResponse<?> response) {
        System.out.println("REQUEST: " + request);
        response.header("abc", "xyz");
    }

    @ResponseFilter
    public Publisher<MutableHttpResponse<?>> simpleResponseReactive(MutableHttpResponse<?> response) {
        return Mono.from(service.fetchSomeReactive()).map(val -> {
            response.header("abc", val);
            return response;
        });
    }

    @ResponseFilter
    public CompletableFuture<MutableHttpResponse<?>> simpleResponseFuture(MutableHttpResponse<?> response) {
        return service.fetchSomeFuture().thenApply(val -> {
            response.header("abc", val);
            return response;
        });
    }

    // Kotlin coroutines

    @ResponseFilter
    public HttpResponse<?> overrideResponse(MutableHttpResponse<?> response) {
        return new HttpResponseWrapper<>(response);
    }

    interface MyExternalService {
        Publisher<String> fetchSomeReactive();
        CompletableFuture<String> fetchSomeFuture();
    }

}

One HTTP filter would be able to contain multiple methods.

The open question is if we will support further filtering of the filter methods by annotating by @Get, @Post etc.

WDYT @micronaut-projects/core-developers

@dstepanov dstepanov added the status: next major version The issue will be considered for the next major version label Oct 17, 2022
@dstepanov dstepanov added this to the 4.0.0 milestone Oct 17, 2022
@dstepanov dstepanov self-assigned this Oct 17, 2022
@sdelamo
Copy link
Contributor

sdelamo commented Oct 18, 2022

Are you proposing to support @RequestFilter and @ResponseFilterin addition to our current support classes annotated with @Filter which implement HttpServerFilter?

@sdelamo
Copy link
Contributor

sdelamo commented Oct 18, 2022

What would happen for methods using imperative programming?

 @RequestFilter
 public void loggingRequestFilter(HttpRequest<?> request) {

Will Micronaut Framework execute these methods in the netty event pool?

@sdelamo
Copy link
Contributor

sdelamo commented Oct 18, 2022

I assume we will create a list of filter composed by old fashion filters (classes annotated with @Filter which implement HttpServerFilter) and filter methods (methods annotated with @RequestFilter and @ResponseFilter. We will sort them all with OrderUtils and apply them. Is this correct?

@sdelamo
Copy link
Contributor

sdelamo commented Oct 18, 2022

Could not we do this for 3.9.0 or 3.10.0 ? Do we need Micronaut Framework 4.0 for this feature?

@dstepanov
Copy link
Contributor Author

Are you proposing to support @RequestFilter and @ResponseFilterin addition to our current support classes annotated with @Filter which implement HttpServerFilter?

Probably yes, but internally there is going to be another filter abstraction that can avoid reactive code.
The best would be to remove the existing filter, but it might be a significant change.

What would happen for methods using imperative programming?

 @RequestFilter
 public void loggingRequestFilter(HttpRequest<?> request) {

Will Micronaut Framework execute these methods in the netty event pool?

Yes, just like it's now.

I assume we will create a list of filter composed by old fashion filters (classes annotated with @Filter which implement HttpServerFilter) and filter methods (methods annotated with @RequestFilter and @ResponseFilter. We will sort them all with OrderUtils and apply them. Is this correct?

To properly implement ordering, we would need to have two sets of filters, request and response, and combining it with the original HttpServerFilter might be a bit complicated.

Could not we do this for 3.9.0 or 3.10.0 ? Do we need Micronaut Framework 4.0 for this feature?

I want to combine it with a refactoring of the HTTP processing so it would be better to target 4.0.

@yawkat
Copy link
Member

yawkat commented Oct 18, 2022

re: "wraparound filters":

we could offer a blocking alternative to wraparound filters as well:

@ExecuteOn(BLOCKING)
HttpResponse filter(request, chain) {
    // do some work before
    HttpResponse response = chain.proceed(request);
    // do some work after
    return response;
}

This is a good use case for loom.

@sdelamo
Copy link
Contributor

sdelamo commented Oct 19, 2022

Are you proposing to support @RequestFilter and @ResponseFilterin addition to our current support classes annotated with @Filter which implement HttpServerFilter?

Probably yes, but internally there is going to be another filter abstraction that can avoid reactive code. The best would be to remove the existing filter, but it might be a significant change.

I think we need to keep classes annotated with @Filter which implement HttpServerFilter at least until Micronaut Framework 5.0.

@jameskleeh
Copy link
Contributor

I think this would be a mistake because of the number of edge cases will be high and it will likely add latency to the server.

It's also more difficult to reason about what is possible given there is no API and it's up to the user to read the documentation to understand what is possible and/or trial and error.

This will require a lot of compile time validation to ensure the arguments and return type are compatible. What happens when a String argument is present? Do we support @QueryValue and the such? What happens when a String is returned?

You could create an imperative version of filters pretty easily and make the functionality easy to understand with a normal API

@dstepanov
Copy link
Contributor Author

I think this would be a mistake because of the number of edge cases will be high and it will likely add latency to the server.

This will require a lot of compile time validation to ensure the arguments and return type are compatible. What happens when a String argument is present? Do we support @QueryValue and the such? What happens when a String is returned?

My original idea was to support more argument binding and path filtering but I think that would have been overkill, the only supported arguments and return values would be the request/response. The implementation should be simple.

It's also more difficult to reason about what is possible given there is no API and it's up to the user to read the documentation to understand what is possible and/or trial and error.

We have the same approach as the controllers, so it should be easy to understand.

You could create an imperative version of filters pretty easily and make the functionality easy to understand with a normal API

We want to support using imperative return types and Reactive-Streams, Java Flow, Java Futures, Kotlin Coroutines. It's not possible to create one API, the approach is the same as with controllers and should be well understood, RESTEasy implements it in the same way.

@graemerocher
Copy link
Contributor

as part of this I would like to see a way we could simplify mutating the incoming body and outgoing body from a filter since that is not easy to do compared to say JAX-RS which has MessageBodyReader and MessageBodyWriter interfaces that are well understood.

@sdelamo
Copy link
Contributor

sdelamo commented May 27, 2023

@yawkat @dstepanov can this be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: next major version The issue will be considered for the next major version
Projects
No open projects
Status: Done
Status: Done
Development

No branches or pull requests

5 participants