smallrye / smallrye-graphql

Implementation for MicroProfile GraphQL
Apache License 2.0
160 stars 92 forks source link

Vertx client implementation requires yasson at runtime #2196

Closed jmini closed 2 months ago

jmini commented 2 months ago

Version 1.5.0 was quite behind, I have tried to update to the newest version and I noticed the need for additional dependencies.

jmartisk commented 2 months ago

Hi @jmini , big thanks for noticing this and sending a PR. Actually, I tend to think that if some extra dependencies are required, then it's a bug. It seems to be fixed when you remove the test scope of Yasson here: https://github.com/smallrye/smallrye-graphql/blob/2.10.0/client/implementation-vertx/pom.xml#L71 After this, I can run the jbang script without any dependencies beyond smallrye-graphql-client-implementation-vertx. Only Yasson is needed, Parsson is brought in by Yasson transitively.

So I would rather have this fixed instead of updating the documentation.

So, could you perhaps change your PR to

jmini commented 2 months ago

I tend to think that if some extra dependencies are required, then it's a bug.

Right now it is the only way to have it working.

With this jbang script:

Client1.java

``` ///usr/bin/env jbang "$0" "$@" ; exit $? //DEPS io.smallrye:smallrye-graphql-client-implementation-vertx:2.10.0 //JAVA 17 import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClient; import io.smallrye.graphql.client.Response; import io.smallrye.graphql.client.vertx.dynamic.VertxDynamicGraphQLClientBuilder; import io.vertx.core.Vertx; // Has a multiline string literal, requires Java 15+! class Client1 { public static void main(String... args) throws Exception { Vertx vertx = Vertx.vertx(); DynamicGraphQLClient client = new VertxDynamicGraphQLClientBuilder() .url("https://countries.trevorblades.com") .vertx(vertx) .build(); try { Response response = client.executeSync(""" query { countries { name } } """); System.out.println(response); } finally { client.close(); vertx.close(); } } } ```

You get:

Exception in thread "main" java.lang.NoClassDefFoundError: jakarta/json/JsonValue
    at io.smallrye.graphql.client.vertx.dynamic.VertxDynamicGraphQLClient.buildRequest(VertxDynamicGraphQLClient.java:388)
    at io.smallrye.graphql.client.vertx.dynamic.VertxDynamicGraphQLClient.executeSync(VertxDynamicGraphQLClient.java:165)
    at Client.main(Client1.java:20)
Caused by: java.lang.ClassNotFoundException: jakarta.json.JsonValue
    at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641)
    at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:520)
    ... 3 more

You are right org.eclipse:yasson:2.0.4 is enough (it probably has a dependency on some JSON Processing library). This works:

Client2.java

``` ///usr/bin/env jbang "$0" "$@" ; exit $? //DEPS io.smallrye:smallrye-graphql-client-implementation-vertx:2.10.0 //DEPS org.eclipse:yasson:2.0.4 //JAVA 17 import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClient; import io.smallrye.graphql.client.Response; import io.smallrye.graphql.client.vertx.dynamic.VertxDynamicGraphQLClientBuilder; import io.vertx.core.Vertx; // Has a multiline string literal, requires Java 15+! class Client2 { public static void main(String... args) throws Exception { Vertx vertx = Vertx.vertx(); DynamicGraphQLClient client = new VertxDynamicGraphQLClientBuilder() .url("https://countries.trevorblades.com") .vertx(vertx) .build(); try { Response response = client.executeSync(""" query { countries { name } } """); System.out.println(response); } finally { client.close(); vertx.close(); } } } ```

jmini commented 2 months ago

I think the correct scope for org.eclipse:yasson in smallrye-graphql-client-parent is runtime, since you do not want to have it at compile time but you need it at test execution and at project runtime time.

I have updated the PR title accordingly.

jmartisk commented 2 months ago

I think the correct scope for org.eclipse:yasson in smallrye-graphql-client-parent is runtime, since you do not want to have it at compile time but you need it at test execution and at project runtime time.

You're right, good point (I think the project contains some more dependencies that could be switched to runtime but we don't care too much)

jmartisk commented 2 months ago

Thanks!