quarkusio/quarkus

ReactiveClientHeadersFactory does not work with internal Uni

Closed this issue · 6 comments

Describe the bug

Hi,

I've already made some comments in the original pull request, which introduced the ReactiveClientHeadersFactory https://github.com/quarkusio/quarkus/pull/21807/files and @Sgitario asked me to open a bug for this issue, which also contains a reproducer.

The issue is that the following code for a ReactiveClientHeadersFactory works using the Uni.createFrom().item() method...

class AuthRequestHeaderFactory : ReactiveClientHeadersFactory() {

    override fun getHeaders(incomingHeaders: MultivaluedMap<String, String>?): Uni<MultivaluedMap<String, String>> {
        return Uni.createFrom().item(
            MultivaluedHashMap<String, String>().apply {
                add("Cache-Control", "no-cache")
                add("Accept-Encoding", "identity")
                add("Authorization", "Basic secret")
            }
        )
    }
}

... while this does not:

class BeverageRequestHeaderFactory(
    @RestClient private val authRestClient: AuthRestClient,
) : ReactiveClientHeadersFactory() {

    override fun getHeaders(incomingHeaders: MultivaluedMap<String, String>?): Uni<MultivaluedMap<String, String>> {
        return authRestClient.fetchClientToken().onItem().transform {
            MultivaluedHashMap<String, String>().apply {
                add(HttpHeaders.CACHE_CONTROL, "no-cache")
                add(HttpHeaders.AUTHORIZATION, "${it.tokenType} ${it.accessToken}")
            }
        }
    }
}

Actually the latter tries to use the auth token, which is received by the rest client, which uses the ReactiveClientHeadersFactory from the first snippet.

The code, which reproduces the issue can be found here:

https://github.com/SimonScholz/ReactiveClientHeadersFactory/blob/b1a7a07c13afa9168b8897653fa27ce0f4cf3be3/src/test/kotlin/io/github/simonscholz/ReactiveResourceTest.kt#L82

The test case shows that the auth rest client can be called in a proper manner by using the AuthRequestHeaderFactory from above, while BeverageRequestHeaderFactory does not work.

When printing all requests, which reached out towards Wiremock, which is used for testing, we can see that also for the BeverageRestClient the values from the AuthRequestHeaderFactory is being used, which is wrong.
It should've been using the BeverageRequestHeaderFactory, which is specified via @RegisterClientHeaders(BeverageRequestHeaderFactory::class) in the BeverageRestClient class.

Please let me know if something is still unclear or if you have questions...

Thanks a lot in advance for your support. :-)

Expected behavior

The expected behavior would be that the BeverageRequestHeaderFactory properly sets the header values for the BeverageRestClient in the reproducer example.

Actual behavior

Unfortunately it seems that the values from the AuthRequestHeaderFactory are being used for the BeverageRestClient as well, even though it should pick up the header value from the BeverageRequestHeaderFactory.

The weird thing also is that if I make the fetchClientToken call blocking it works....

How to Reproduce?

I've created this sample code in order to reproduce the issue we're facing:

https://github.com/SimonScholz/ReactiveClientHeadersFactory/blob/b1a7a07c13afa9168b8897653fa27ce0f4cf3be3/src/test/kotlin/io/github/simonscholz/ReactiveResourceTest.kt#L82

In this test case in line 82 I print all requests, which were made towards Wiremock.
With that in place I was able to see that the wrong header values are being used for the BeverageRestClient.

Feel free to clone the Github repository, I've created, and run the test.

Output of uname -a or ver

No response

Output of java -version

Java 17.0.1 Temurin

GraalVM version (if different from Java)

OpenJDK Runtime Environment GraalVM CE 21.3.0

Quarkus version or git rev

2.7.1

Build tool (ie. output of mvnw --version or gradlew --version)

Gradle 7.3.3

Additional information

I've already made some comments in the original PR, which introduced the ReactiveClientHeadersFactory.
Also see https://github.com/quarkusio/quarkus/pull/21807/files

@SimonScholz can you try Quarkus from the current main branch? It should be fixed in #24099
I don't know yet if this will be backported to 2.7

@michalszynkiewicz Thanks for the quick response. Can I get the snapshot version of the main branch from a certain artifact repository?
Or do I have to build it locally and deploy it to my local maven repository?

I have never tried it but guessing from Github Actions we have, this can be the repository to use: https://oss.sonatype.org/content/repositories/snapshots

I have never tried it but guessing from Github Actions we have, this can be the repository to use: https://oss.sonatype.org/content/repositories/snapshots

Yes, this is built nightly

I am going to assume this was fixed. If it's not, please comment and we can revisit the issue.