-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Rest Client Reactive: Add reactive flavor for ClientHeadersFactory #21807
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
86 changes: 86 additions & 0 deletions
86
...t/java/io/quarkus/rest/client/reactive/headers/ReactiveClientHeadersFromProviderTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
package io.quarkus.rest.client.reactive.headers; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
import java.net.URI; | ||
|
||
import javax.enterprise.context.ApplicationScoped; | ||
import javax.inject.Inject; | ||
import javax.ws.rs.GET; | ||
import javax.ws.rs.HeaderParam; | ||
import javax.ws.rs.Path; | ||
import javax.ws.rs.core.MultivaluedHashMap; | ||
import javax.ws.rs.core.MultivaluedMap; | ||
|
||
import org.eclipse.microprofile.rest.client.RestClientBuilder; | ||
import org.eclipse.microprofile.rest.client.annotation.RegisterClientHeaders; | ||
import org.jboss.shrinkwrap.api.asset.StringAsset; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
||
import io.quarkus.rest.client.reactive.ReactiveClientHeadersFactory; | ||
import io.quarkus.test.QuarkusUnitTest; | ||
import io.quarkus.test.common.http.TestHTTPResource; | ||
import io.smallrye.common.annotation.Blocking; | ||
import io.smallrye.mutiny.Uni; | ||
|
||
public class ReactiveClientHeadersFromProviderTest { | ||
private static final String HEADER_NAME = "my-header"; | ||
private static final String HEADER_VALUE = "oifajrofijaeoir5gjaoasfaxcvcz"; | ||
|
||
@TestHTTPResource | ||
URI baseUri; | ||
|
||
@RegisterExtension | ||
static final QuarkusUnitTest config = new QuarkusUnitTest() | ||
.withApplicationRoot((jar) -> jar.addClasses(ReactiveClientHeadersFromProviderTest.Client.class) | ||
.addAsResource( | ||
new StringAsset("my.property-value=" + HEADER_VALUE), | ||
"application.properties")); | ||
|
||
@Test | ||
void shouldSetHeaderFromProperties() { | ||
ReactiveClientHeadersFromProviderTest.Client client = RestClientBuilder.newBuilder().baseUri(baseUri) | ||
.build(ReactiveClientHeadersFromProviderTest.Client.class); | ||
|
||
assertThat(client.getWithHeader()).isEqualTo(HEADER_VALUE); | ||
} | ||
|
||
@Path("/") | ||
@ApplicationScoped | ||
public static class Resource { | ||
@GET | ||
public String returnHeaderValue(@HeaderParam(HEADER_NAME) String header) { | ||
return header; | ||
} | ||
} | ||
|
||
@ApplicationScoped | ||
public static class Service { | ||
@Blocking | ||
public String getValue() { | ||
return 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 Uni.createFrom().item(() -> { | ||
MultivaluedHashMap<String, String> newHeaders = new MultivaluedHashMap<>(); | ||
newHeaders.add(HEADER_NAME, service.getValue()); | ||
return newHeaders; | ||
}); | ||
} | ||
} | ||
} |
21 changes: 21 additions & 0 deletions
21
...e/runtime/src/main/java/io/quarkus/rest/client/reactive/ReactiveClientHeadersFactory.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package io.quarkus.rest.client.reactive; | ||
|
||
import javax.ws.rs.core.MultivaluedMap; | ||
|
||
import org.eclipse.microprofile.rest.client.ext.ClientHeadersFactory; | ||
|
||
import io.smallrye.mutiny.Uni; | ||
|
||
/** | ||
* Reactive ClientHeadersFactory flavor for Quarkus rest-client reactive extension. | ||
*/ | ||
public abstract class ReactiveClientHeadersFactory implements ClientHeadersFactory { | ||
michalszynkiewicz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public abstract Uni<MultivaluedMap<String, String>> getHeaders(MultivaluedMap<String, String> incomingHeaders); | ||
|
||
@Override | ||
public final MultivaluedMap<String, String> update(MultivaluedMap<String, String> incomingHeaders, | ||
MultivaluedMap<String, String> clientOutgoingHeaders) { | ||
throw new RuntimeException( | ||
"Can't call `update` method in a Reactive context. Use `getHeaders` or implement ClientHeadersFactory."); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
Unfortunately this does not work, while this is working:
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.