Injecting into custom ClientRequestFilter does not work with Reactive RestClient

[sberyozkin]

Describe the bug

Reported by Simone DiCola.

For example, injecting a RequestScoped Tokens shipped with quarkus-oidc-client into a custom filter:

@Provider
@Priority(Priorities.AUTHENTICATION)
public class OidcClientRequestCustomFilter implements ClientRequestFilter {

    @Inject
    Tokens tokens;

    @Override
    public void filter(ClientRequestContext requestContext) throws IOException {
        requestContext.getHeaders().add(HttpHeaders.AUTHORIZATION, "Bearer " + tokens.getAccessToken());
    }
}

works with quarkus-rest-client but it is null with quarkus-rest-client-reactive.

[quarkus-bot]

/cc @FroMage, @geoand, @stuartwdouglas

[geoand]

Good point. We should fix this one. I'll take a later this week

[sberyozkin]

Hey @geoand - thanks - let me know please if i can help somehow - I was just typing a line suggesting that it should be reproducible with a custom test filter injecting some test bean (no need to use OidcClientFilter) - perhaps I can help with a reproducer, etc

[geoand]

Thanks for the offer @sberyozkin :).

I know exactly what this doesn't work and I am already refactoring the client code a little for #16615, so this will fit it nicely one I'm done with the other issue

[sberyozkin]

@geoand, Np at all, cool, great you are already aware of where the problem is :-)

[geoand]

Actually, I tried this and could not reproduce this issue.

I took https://github.com/quarkusio/quarkus-quickstarts/tree/development/rest-client-reactive-quickstart and added:

@Provider
public class MyFilter implements ClientRequestFilter {

    @Inject
    HttpServerRequest httpServerRequest;

    @Override
    public void filter(ClientRequestContext requestContext) throws IOException {
        System.out.println(httpServerRequest.absoluteURI());
    }
}

while also adding @RegisterProvider(MyFilter.class) on CountriesService and everything worked just fine

[sberyozkin]

Hi @geoand thanks, let me try to add a reactive rest client/oidc filter integration test based on your enhancement earlier and see what happens :-)

[geoand]

Thanks @sberyozkin. If you come up with a reproducer of the problem, I'd love to see it :)

[sberyozkin]

Hey @geoand, please see #16658. It gives me NPE - the curious thing is, if I add @Provider to it - NPE is gone - but the filter is not invoked. But this filter is registered with @RegisterProvider.
FYI, I've tried injecting HttpServerRequest and it also gives NPE

[geoand]

So indeed @Provider is needed to make one of these filters work as a CDI bean.
The JAX-RS spec mentions:

An extension interface implemented by client request filters. Filters implementing this interface MUST be annotated with @Provider. This type of filters is supported only as part of the Client API.

from https://docs.oracle.com/javaee/7/api/javax/ws/rs/client/ClientRequestFilter.html

Was this not the case with the old rest-client?

[sberyozkin]

It works OK here since it is registered as a provider here.

I don't really mind adding @Provider - as thought it was mainly for the auto-discovery - even in this test it is recognized as ClientRequestFilter without @Provider - the CDI aspect is an extra one... But yeah, if I add @Provider then @RegisterProvider(OidcClientCustomRequestFillter.class) is ignored. Let me retry it

[geoand]

@michalszynkiewicz I think that we should register the class in @RegisterProvider as bean. I assume that's what the old rest-client does

[sberyozkin]

@geoand OK, sorry, the injection works with @Provider - I was confused as the test endpoint is returning an empty principal so assumed the filter was skipped - I'll ping on Zulip.

[geoand]

NP!

[sberyozkin]

Actually the test passes now - it is just that the reactive client is a bit sensitive to the lack of @Produces("text/plain") - probably because it assumes application/json by default... So all is good there - if you'd like please close this issue - unless you'd like to track a possible @RegisterProvider(MyProvider.class) + CDI enhancement here...

Cheers

[geoand]

Yeah, let's keep this open for the CDI enhancement

[geoand]

#16673 contains the CDI enhancement