Rest Client Reactive: Add reactive flavor for ClientHeadersFactory #21807

Sgitario · 2021-11-30T06:19:27Z

Added reactive flavor of ClientHeadersFactory that allows doing blocking operations. For example:

package org.acme.rest.client;

import org.eclipse.microprofile.rest.client.ext.ClientHeadersFactory;

import javax.enterprise.context.ApplicationScoped;
import javax.ws.rs.core.MultivaluedHashMap;
import javax.ws.rs.core.MultivaluedMap;
import java.util.UUID;

@ApplicationScoped
public class GetTokenReactiveClientHeadersFactory extends ReactiveClientHeadersFactory {

    @Inject
    Service service;

    @Override
    public Uni<MultivaluedMap<String, String>> getHeaders(MultivaluedMap<String, String> incomingHeaders) {
        return Uni.createFrom().item(() -> {
            MultivaluedHashMap<String, String> newHeaders = new MultivaluedHashMap<>();
            // perform blocking call
            newHeaders.add(HEADER_NAME, service.getToken());
            return newHeaders;
        });
    }
}

Resolves #20001

Sgitario · 2021-11-30T06:32:09Z

@michalszynkiewicz this PR proposes to create a version of ClientHeadersFactory for reactive by implementing the ClientHeadersFactory interface.

The benefit is that we can reuse the same annotation (@RegisterClientHeaders) and features that were already in place at MicroProfileRestClientRequestFilter.java.
However, the new reactive flavor of ClientHeadersFactory, ReactiveClientHeadersFactory will NOT be an interface, but an abstract class, and also I had to implement a fake update method (see https://github.com/quarkusio/quarkus/pull/21807/files#diff-588a0da8278ac069a69d2db5ef55e2c8e165b1995e667b24550731e53310afd7R16). I'm throwing an exception to make a strong notice that this method should never be called plus I also disallow users to overwrite it.

In summary, by extending ClientHeadersFactory, we reuse everything that was in place at MicroProfileRestClientRequestFilter.java, but we are forced to implement the update method in ReactiveClientHeadersFactory.

If you're not happy with, the alternative is to:

Create a new annotation @RegisterReactiveClientHeaders, with a default implementation DefaultReactiveClientHeadersFactoryImpl that does nothing,
Add the reactive filter and copy/somehow reuse everything that was in MicroProfileRestClientRequestFilter.java.
Update the deployments to work with the new annotation which will imply to duplicate a few things.

Let me know your thoughts about it.

quarkus-bot · 2021-11-30T06:38:34Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `6d054a9`

Status	Name	Step	Failures	Logs	Raw logs
✖	Initial JDK 11 Build	`Build`	Failures	Logs	Raw logs

Failures

⚙️ Initial JDK 11 Build #

- Failing: extensions/resteasy-reactive/rest-client-reactive/runtime 
! Skipped: devtools/bom-descriptor-json docs extensions/oidc-client-reactive-filter/deployment and 12 more

📦 extensions/resteasy-reactive/rest-client-reactive/runtime

✖ Failed to execute goal net.revelc.code.formatter:formatter-maven-plugin:2.17.0:validate (default) on project quarkus-rest-client-reactive: File '/home/runner/work/quarkus/quarkus/extensions/resteasy-reactive/rest-client-reactive/runtime/src/main/java/io/quarkus/rest/client/reactive/ReactiveClientHeadersFactory.java' has not been previously formatted. Please format file and commit before running validation!

quarkus-bot · 2021-11-30T07:16:16Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `fffde59`

Status	Name	Step	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Failures	Logs	Raw logs
✖	JVM Tests - JDK 11 Windows	`Build`	Failures	Logs	Raw logs
✖	JVM Tests - JDK 17	`Build`	Failures	Logs	Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/resteasy-reactive/rest-client-reactive/deployment 
! Skipped: extensions/oidc-client-reactive-filter/deployment extensions/resteasy-reactive/rest-client-reactive-jackson/deployment integration-tests/micrometer-prometheus and 5 more

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

✖ io.quarkus.rest.client.reactive.ClientAndServerSharingResponseTest.test line 46 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

✖ io.quarkus.rest.client.reactive.stork.StorkDevModeTest.shouldModifyStorkSettings line 67 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/resteasy-reactive/rest-client-reactive/deployment 
! Skipped: extensions/oidc-client-reactive-filter/deployment extensions/resteasy-reactive/rest-client-reactive-jackson/deployment integration-tests/micrometer-prometheus and 5 more

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

✖ io.quarkus.rest.client.reactive.ClientAndServerSharingResponseTest.test line 46 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

✖ io.quarkus.rest.client.reactive.stork.StorkDevModeTest.shouldModifyStorkSettings line 67 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

⚙️ JVM Tests - JDK 17 #

- Failing: extensions/resteasy-reactive/rest-client-reactive/deployment 
! Skipped: extensions/oidc-client-reactive-filter/deployment extensions/resteasy-reactive/rest-client-reactive-jackson/deployment integration-tests/micrometer-prometheus and 5 more

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

✖ io.quarkus.rest.client.reactive.ClientAndServerSharingResponseTest.test line 46 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

✖ io.quarkus.rest.client.reactive.stork.StorkDevModeTest.shouldModifyStorkSettings line 67 - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Response body doesn't match expectation.

...tive/runtime/src/main/java/io/quarkus/rest/client/reactive/ReactiveClientHeadersFactory.java

michalszynkiewicz · 2021-11-30T13:33:14Z

...c/main/java/io/quarkus/rest/client/reactive/runtime/MicroProfileRestClientRequestFilter.java

@@ -63,7 +65,21 @@ public void filter(ClientRequestContext requestContext) {
        }

        if (clientHeadersFactory != null) {
-            incomingHeaders = clientHeadersFactory.update(incomingHeaders, headers);
+            if (clientHeadersFactory instanceof ReactiveClientHeadersFactory
+                    && requestContext instanceof ResteasyReactiveClientRequestContext) {


this filter should always work with RestEasyReactiveClientRequestContext, no? I don't think we need to check it with instanceof

I think so... but I thought we could be extra safe here. However, will remove the instanceof.

@Inject

Added reactive flavor of `ClientHeadersFactory` that allows doing blocking operations. For example: [source, java] ---- package org.acme.rest.client; import org.eclipse.microprofile.rest.client.ext.ClientHeadersFactory; import javax.enterprise.context.ApplicationScoped; import javax.ws.rs.core.MultivaluedHashMap; import javax.ws.rs.core.MultivaluedMap; import java.util.UUID; @ApplicationScoped public class GetTokenReactiveClientHeadersFactory extends ReactiveClientHeadersFactory { @Inject Service service; @OverRide public Uni<MultivaluedMap<String, String>> getHeaders(MultivaluedMap<String, String> incomingHeaders) { return Uni.createFrom().item(() -> { MultivaluedHashMap<String, String> newHeaders = new MultivaluedHashMap<>(); // perform blocking call newHeaders.add(HEADER_NAME, service.getToken()); return newHeaders; }); } } ---- Fixes quarkusio#20001

michalszynkiewicz

thanks @Sgitario !

SimonScholz · 2022-02-14T23:02:24Z

docs/src/main/asciidoc/rest-client-reactive.adoc

+        return Uni.createFrom().item(() -> {
+            MultivaluedHashMap<String, String> newHeaders = new MultivaluedHashMap<>();
+            // perform blocking call
+            newHeaders.add(HEADER_NAME, service.getToken());


Hi,
I am sorry that I kind of hijack this already merged PR, but IMHO the ReactiveClientHeadersFactory does not work as expected.
Why has the service.getToken() to be a blocking call?
In my case I wanted to call the auth server where I obtain a token also in a reactive manner by using an reactive rest client.
Therefore my code would look like this:

override fun getHeaders(incomingHeaders: MultivaluedMap<String, String>?): Uni<MultivaluedMap<String, String>> { // Utilizing the reactive mutiny api :-), which unfortunately does not work.... return clientTokenFetcher.getClientToken().onItem().transform { MultivaluedHashMap<String, String>().apply { add(HttpHeaders.CACHE_CONTROL, "no-cache") add(HttpHeaders.CONTENT_TYPE, "text/plain") add(HttpHeaders.AUTHORIZATION, it.authHeader) } } }

Unfortunately this does not work, while this is working:

override fun getHeaders(incomingHeaders: MultivaluedMap<String, String>?): Uni<MultivaluedMap<String, String>> { return Uni.createFrom().item( MultivaluedHashMap<String, String>().apply { add(HttpHeaders.CACHE_CONTROL, "no-cache") add(HttpHeaders.CONTENT_TYPE, "text/plain") // Make this a blocking call ;-( add(HttpHeaders.AUTHORIZATION, clientTokenFetcher.getClientToken().await().atMost(Duration.ofMillis(500)).authHeader) } ) }

Can someone please tell me how to stay fully reactive and make it work without the blocking within the Uni?

The code above is written in kotlin, but I could also convert it into Java if that helps you...

The service.getToken() does not have to be a blocking call. This example was only intended to explain how to inject headers from a blocking service to Resteasy Rest Client.

Also, I tried to reproduce your scenario and it worked for me:

@ApplicationScoped public static class Service { public Uni<String> getValue() { return Uni.createFrom().item(HEADER_VALUE); } } @RegisterClientHeaders(CustomReactiveClientHeadersFactory.class) public interface Client { @GET String getWithHeader(); } public static class CustomReactiveClientHeadersFactory extends ReactiveClientHeadersFactory { @Inject Service service; @Override public Uni<MultivaluedMap<String, String>> getHeaders(MultivaluedMap<String, String> incomingHeaders) { return service.getValue().onItem().transform(value -> { MultivaluedHashMap<String, String> newHeaders = new MultivaluedHashMap<>(); newHeaders.add(HEADER_NAME, value); return newHeaders; }); } }

This is verified using this test https://github.com/quarkusio/quarkus/blob/main/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/headers/ReactiveClientHeadersFromProviderTest.java

Therefore, it would be good to open an issue with a reproducer, so we can exactly see what is going on.

Hi @Sgitario ,

thanks for your quick response:
Here you can find an example which reproduces my issue:
https://github.com/SimonScholz/ReactiveClientHeadersFactory/blob/b1a7a07c13afa9168b8897653fa27ce0f4cf3be3/src/test/kotlin/io/github/simonscholz/ReactiveResourceTest.kt#L82

In this example I'd use a reactive rest client in order to obtain a token, which is then used by the BeverageRequestHeaderFactory to generate a proper Authorization header.
The ReactiveResourceTest shows that this does not work.
I'll also create an issue later on, but wanted to inform you about my example in advance.

Thanks and regards,

Simon

Hi @Sgitario,

FYI, I've now finally created an issue here: #24153

Please let me know, if I can do more or if you have any questions.

quarkus-bot bot added area/documentation area/rest labels Nov 30, 2021

Sgitario force-pushed the reactive_ClientHeadersFactory branch from 6d054a9 to fffde59 Compare November 30, 2021 06:43

Sgitario force-pushed the reactive_ClientHeadersFactory branch from fffde59 to 9ebba56 Compare November 30, 2021 07:29

geoand requested a review from michalszynkiewicz November 30, 2021 08:19

geoand removed the area/documentation label Nov 30, 2021

michalszynkiewicz reviewed Nov 30, 2021

View reviewed changes

...tive/runtime/src/main/java/io/quarkus/rest/client/reactive/ReactiveClientHeadersFactory.java Outdated Show resolved Hide resolved

michalszynkiewicz reviewed Nov 30, 2021

View reviewed changes

...tive/runtime/src/main/java/io/quarkus/rest/client/reactive/ReactiveClientHeadersFactory.java Show resolved Hide resolved

michalszynkiewicz reviewed Nov 30, 2021

View reviewed changes

Sgitario force-pushed the reactive_ClientHeadersFactory branch from 9ebba56 to 87be3ed Compare November 30, 2021 13:46

quarkus-bot bot added the area/documentation label Nov 30, 2021

Sgitario requested a review from michalszynkiewicz November 30, 2021 13:47

michalszynkiewicz approved these changes Nov 30, 2021

View reviewed changes

geoand merged commit d8199b3 into quarkusio:main Nov 30, 2021

quarkus-bot bot added this to the 2.6 - main milestone Nov 30, 2021

Sgitario deleted the reactive_ClientHeadersFactory branch November 30, 2021 19:09

SimonScholz reviewed Feb 14, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rest Client Reactive: Add reactive flavor for ClientHeadersFactory #21807

Rest Client Reactive: Add reactive flavor for ClientHeadersFactory #21807

Sgitario commented Nov 30, 2021 •

edited by geoand

Loading

Sgitario commented Nov 30, 2021

quarkus-bot bot commented Nov 30, 2021 •

edited

Loading

Failing Jobs - Building `6d054a9`

Failures

⚙️ Initial JDK 11 Build #

📦 extensions/resteasy-reactive/rest-client-reactive/runtime

quarkus-bot bot commented Nov 30, 2021 •

edited

Loading

Failing Jobs - Building `fffde59`

Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

⚙️ JVM Tests - JDK 17 #

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

michalszynkiewicz Nov 30, 2021

Sgitario Nov 30, 2021

michalszynkiewicz left a comment

SimonScholz Feb 14, 2022

Sgitario Feb 15, 2022

SimonScholz Feb 23, 2022

SimonScholz Mar 7, 2022

Rest Client Reactive: Add reactive flavor for ClientHeadersFactory #21807

Rest Client Reactive: Add reactive flavor for ClientHeadersFactory #21807

Conversation

Sgitario commented Nov 30, 2021 • edited by geoand Loading

Sgitario commented Nov 30, 2021

quarkus-bot bot commented Nov 30, 2021 • edited Loading

Failing Jobs - Building 6d054a9

Failures

⚙️ Initial JDK 11 Build #

📦 extensions/resteasy-reactive/rest-client-reactive/runtime

quarkus-bot bot commented Nov 30, 2021 • edited Loading

Failing Jobs - Building fffde59

Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

⚙️ JVM Tests - JDK 11 Windows #

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

⚙️ JVM Tests - JDK 17 #

📦 extensions/resteasy-reactive/rest-client-reactive/deployment

michalszynkiewicz Nov 30, 2021

Choose a reason for hiding this comment

Sgitario Nov 30, 2021

Choose a reason for hiding this comment

michalszynkiewicz left a comment

Choose a reason for hiding this comment

SimonScholz Feb 14, 2022

Choose a reason for hiding this comment

Sgitario Feb 15, 2022

Choose a reason for hiding this comment

SimonScholz Feb 23, 2022

Choose a reason for hiding this comment

SimonScholz Mar 7, 2022

Choose a reason for hiding this comment

Sgitario commented Nov 30, 2021 •

edited by geoand

Loading

quarkus-bot bot commented Nov 30, 2021 •

edited

Loading

Failing Jobs - Building `6d054a9`

quarkus-bot bot commented Nov 30, 2021 •

edited

Loading

Failing Jobs - Building `fffde59`