oidc-client-graphql tests are failing with NPE

[sberyozkin]

Describe the bug

#36375 introduced oidc-client-grapghql, some PRs are failing with:

2023-10-13T00:41:14.4473397Z Caused by: java.lang.NullPointerException: Cannot invoke "java.util.Map.put(Object, Object)" because "dynamicHeaders" is null
2023-10-13T00:41:14.4475762Z 	at io.quarkus.oidc.client.graphql.runtime.OidcGraphQLClientIntegrationRecorder.lambda$enhanceGraphQLClientConfigurationWithOidc$0(OidcGraphQLClientIntegrationRecorder.java:25)
2023-10-13T00:41:14.4477670Z 	at java.base/java.util.HashMap.forEach(HashMap.java:1421)
2023-10-13T00:41:14.4479641Z 	at io.quarkus.oidc.client.graphql.runtime.OidcGraphQLClientIntegrationRecorder.enhanceGraphQLClientConfigurationWithOidc(OidcGraphQLClientIntegrationRecorder.java:19)
2023-10-13T00:41:14.4482048Z 	at io.quarkus.deployment.steps.OidcGraphQLClientIntegrationProcessor$initialize1724027614.deploy_0(Unknown Source)
2023-10-13T00:41:14.4483872Z 	at io.quarkus.deployment.steps.OidcGraphQLClientIntegrationProcessor$initialize1724027614.deploy(Unknown Source)

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

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

No response

Additional information

No response

[quarkus-bot]

/cc @jmartisk (graphql), @pedroigor (oidc), @phillip-kruger (graphql)

[sberyozkin]

It all passes on my laptop:

2023-10-13 13:38:36,484 INFO  [io.qua.oid.dep.dev.OidcDevUIProcessor] (build-36) OIDC Dev Console: discovering the provider metadata at http://localhost:32773/auth/realms/quarkus/.well-known/openid-configuration
2023-10-13 13:38:38,514 INFO  [io.quarkus] (Quarkus Main Thread) quarkus-oidc-client-graphql-deployment 999-SNAPSHOT on JVM (powered by Quarkus 999-SNAPSHOT) started in 2.605s. Listening on: http://localhost:8080
2023-10-13 13:38:38,515 INFO  [io.quarkus] (Quarkus Main Thread) Profile dev activated. Live Coding activated.
2023-10-13 13:38:38,515 INFO  [io.quarkus] (Quarkus Main Thread) Installed features: [cdi, hibernate-validator, oidc, oidc-client, oidc-client-graphql-client-integration, resteasy-reactive, security, smallrye-context-propagation, smallrye-graphql, smallrye-graphql-client, vertx]
2023-10-13 13:38:38,793 INFO  [io.quarkus] (Quarkus Main Thread) quarkus-oidc-client-graphql-deployment stopped in 0.006s
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 35.90 s -- in io.quarkus.oidc.client.graphql.GraphQLClientUsingOidcClientTest

[gsmet]

As I said in the comment where I pinged @jmartisk , it's probably a race condition (that happens more regularly on CI because it's slow).

[gsmet]

I created a PR to disable this test as it's very disruptive: #36493 .

[gsmet]

A couple of comments here:

• I think GraphQLClientsConfiguration.clients should be a CHM, not a HM
• It would be preferable to initialize GraphQLClientConfiguration in a fully immutable way (typically with a builder)

[gsmet]

/cc @jmartisk ^

[sberyozkin]

Looks like some more work is needed at the smallrye-graphql level, I've removed the backport label from the PR which introduced the extension, targeting 3.6.0 looks safer at this stage

[sberyozkin]

I suppose reverting the PR will hopefully not be necessary, Jan will be back tomorrow, so he'll have enough time to fix it and re-enable to tests to confirm.

[jmartisk]

A couple of comments here:

* I think `GraphQLClientsConfiguration.clients` should be a CHM, not a HM

I think that shouldn't be necessary, as long as recorder steps are executed sequentially and not in parallel

* It would be preferable to initialize `GraphQLClientConfiguration` in a fully immutable way (typically with a builder)

That would be nice but quite unwieldy given that three different projects write to it (two Quarkus extensions and smallrye-graphql itself)

The problem here arises because we basically depend on the order of recorder steps. If

quarkus/extensions/smallrye-graphql-client/runtime/src/main/java/io/quarkus/smallrye/graphql/client/runtime/SmallRyeGraphQLClientRecorder.java

Line 47 in 0a8efce

public void initializeConfigurationMergerBean() {

executes first and

quarkus/extensions/smallrye-graphql-client/runtime/src/main/java/io/quarkus/smallrye/graphql/client/runtime/SmallRyeGraphQLClientRecorder.java

Line 26 in 0a8efce

public void setTypesafeApiClasses(List<String> apiClassNames) {

second, then all works well. But if they execute in the opposite order, then due to my oversight, a client configuration is created where dynamicHeaders == null, and when later merging the OIDC client part into that config, it causes the NPE. I'm going to fix it on the smallrye side to make sure that dynamicHeaders is never null. I want the client configuration merging logic to be so robust that the order of steps doesn't matter, but in this one specific case it did matter, so I'll fix it.

[jmartisk]

With smallrye/smallrye-graphql#1939 I have run the test 140 times and got zero failures

[jmartisk]

Let's keep it open until we upgrade to a fixed smallrye-graphql version (and re-enable the test)