Skip to content
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

Vertx client implementation requires yasson at runtime #2196

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

jmini
Copy link
Contributor

@jmini jmini commented Sep 24, 2024

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
Copy link
Member

jmartisk commented Sep 25, 2024

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

  • remove the line that I linked
  • in the docs, for now we will have to stick with version 2.10.0 until we put out a new release, so the Yasson dependency will still need to be mentioned - maybe you could add a sentence that 2.10.0 has a bug that requires you to add Yasson, and right after releasing 2.11.0, I would then remove this note?

@jmini
Copy link
Contributor Author

jmini commented Sep 25, 2024

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 jmini changed the title Update standalone client JBang example Vertx client implementation requires yasson at runtime Sep 25, 2024
@jmini
Copy link
Contributor Author

jmini commented Sep 25, 2024

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.

///usr/bin/env jbang "$0" "$@" ; exit $?
//DEPS io.smallrye:smallrye-graphql-client-implementation-vertx:1.5.0
//DEPS io.smallrye:smallrye-graphql-client-implementation-vertx:RELEASE
Copy link
Member

@jmartisk jmartisk Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that JBang supports this version pointer, I get

[jbang] [ERROR] Could not resolve dependencies: Failed to collect dependencies at io.smallrye:smallrye-graphql-client-implementation-vertx:jar:RELEASE

when running it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which jbang version do you have?
With version 0.117.1, this works well for me.

My Client3.java starts with:

///usr/bin/env jbang "$0" "$@" ; exit $?
//DEPS io.smallrye:smallrye-graphql-client-implementation-vertx:RELEASE
//DEPS org.eclipse:yasson:2.0.4
...

And it works well.

Alsojbang info tools Client3.java indicates correct dependencies and resolvedDependencies

Output of the info command

{
  "originalResource": "Client3.java",
  "backingResource": "/__USER_HOME__/tmp/jbang-tmp/Client3.java",
  "applicationJar": "/__USER_HOME__/.jbang/cache/jars/Client3.java.9a5cdfd255acacf1625c98fa97313d79b0bdc876dde5018f2393c2e79cf66254/Client3.jar",
  "dependencies": [
    "io.smallrye:smallrye-graphql-client-implementation-vertx:RELEASE",
    "org.eclipse:yasson:2.0.4"
  ],
  "resolvedDependencies": [
    "/__USER_HOME__/.m2/repository/io/smallrye/smallrye-graphql-client-implementation-vertx/2.10.0/smallrye-graphql-client-implementation-vertx-2.10.0.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/smallrye-graphql-client/2.10.0/smallrye-graphql-client-2.10.0.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/smallrye-graphql-api/2.10.0/smallrye-graphql-api-2.10.0.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/common/smallrye-common-annotation/2.1.0/smallrye-common-annotation-2.1.0.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/smallrye-graphql-client-api/2.10.0/smallrye-graphql-client-api-2.10.0.jar",
    "/__USER_HOME__/.m2/repository/org/eclipse/microprofile/graphql/microprofile-graphql-api/2.0/microprofile-graphql-api-2.0.jar",
    "/__USER_HOME__/.m2/repository/jakarta/enterprise/jakarta.enterprise.cdi-api/4.0.1/jakarta.enterprise.cdi-api-4.0.1.jar",
    "/__USER_HOME__/.m2/repository/jakarta/enterprise/jakarta.enterprise.lang-model/4.0.1/jakarta.enterprise.lang-model-4.0.1.jar",
    "/__USER_HOME__/.m2/repository/jakarta/annotation/jakarta.annotation-api/2.1.0/jakarta.annotation-api-2.1.0.jar",
    "/__USER_HOME__/.m2/repository/jakarta/el/jakarta.el-api/5.0.0/jakarta.el-api-5.0.0.jar",
    "/__USER_HOME__/.m2/repository/jakarta/interceptor/jakarta.interceptor-api/2.1.0/jakarta.interceptor-api-2.1.0.jar",
    "/__USER_HOME__/.m2/repository/jakarta/inject/jakarta.inject-api/2.0.1/jakarta.inject-api-2.0.1.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/reactive/mutiny/2.3.1/mutiny-2.3.1.jar",
    "/__USER_HOME__/.m2/repository/org/jboss/logging/jboss-logging/3.5.3.Final/jboss-logging-3.5.3.Final.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/smallrye-graphql-client-model/2.10.0/smallrye-graphql-client-model-2.10.0.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/config/smallrye-config/3.5.2/smallrye-config-3.5.2.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/config/smallrye-config-core/3.5.2/smallrye-config-core-3.5.2.jar",
    "/__USER_HOME__/.m2/repository/org/eclipse/microprofile/config/microprofile-config-api/3.1/microprofile-config-api-3.1.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/common/smallrye-common-expression/2.2.0/smallrye-common-expression-2.2.0.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/common/smallrye-common-function/2.2.0/smallrye-common-function-2.2.0.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/common/smallrye-common-constraint/2.2.0/smallrye-common-constraint-2.2.0.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/common/smallrye-common-classloader/2.2.0/smallrye-common-classloader-2.2.0.jar",
    "/__USER_HOME__/.m2/repository/org/ow2/asm/asm/9.6/asm-9.6.jar",
    "/__USER_HOME__/.m2/repository/io/smallrye/config/smallrye-config-common/3.5.2/smallrye-config-common-3.5.2.jar",
    "/__USER_HOME__/.m2/repository/io/vertx/vertx-web-client/4.5.4/vertx-web-client-4.5.4.jar",
    "/__USER_HOME__/.m2/repository/io/vertx/vertx-uri-template/4.5.4/vertx-uri-template-4.5.4.jar",
    "/__USER_HOME__/.m2/repository/io/vertx/vertx-web-common/4.5.4/vertx-web-common-4.5.4.jar",
    "/__USER_HOME__/.m2/repository/io/vertx/vertx-auth-common/4.5.4/vertx-auth-common-4.5.4.jar",
    "/__USER_HOME__/.m2/repository/io/vertx/vertx-core/4.5.4/vertx-core-4.5.4.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-common/4.1.107.Final/netty-common-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-buffer/4.1.107.Final/netty-buffer-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-transport/4.1.107.Final/netty-transport-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-handler/4.1.107.Final/netty-handler-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-transport-native-unix-common/4.1.107.Final/netty-transport-native-unix-common-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-codec/4.1.107.Final/netty-codec-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-handler-proxy/4.1.107.Final/netty-handler-proxy-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-codec-socks/4.1.107.Final/netty-codec-socks-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-codec-http/4.1.107.Final/netty-codec-http-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-codec-http2/4.1.107.Final/netty-codec-http2-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-resolver/4.1.107.Final/netty-resolver-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-resolver-dns/4.1.107.Final/netty-resolver-dns-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/io/netty/netty-codec-dns/4.1.107.Final/netty-codec-dns-4.1.107.Final.jar",
    "/__USER_HOME__/.m2/repository/com/fasterxml/jackson/core/jackson-core/2.16.1/jackson-core-2.16.1.jar",
    "/__USER_HOME__/.m2/repository/org/eclipse/yasson/2.0.4/yasson-2.0.4.jar",
    "/__USER_HOME__/.m2/repository/jakarta/json/bind/jakarta.json.bind-api/2.0.0/jakarta.json.bind-api-2.0.0.jar",
    "/__USER_HOME__/.m2/repository/jakarta/json/jakarta.json-api/2.0.0/jakarta.json-api-2.0.0.jar",
    "/__USER_HOME__/.m2/repository/org/glassfish/jakarta.json/2.0.0/jakarta.json-2.0.0-module.jar"
  ],
  "javaVersion": "17",
  "requestedJavaVersion": "17",
  "availableJdkPath": "/__USER_HOME__/.jbang/cache/jdks/17",
  "compileOptions": [
    "-g"
  ],
  "sources": [
    {
      "originalResource": "Client3.java",
      "backingResource": "/__USER_HOME__/tmp/jbang-tmp/Client3.java"
    }
  ]
}

Client3.java

///usr/bin/env jbang "$0" "$@" ; exit $?
//DEPS io.smallrye:smallrye-graphql-client-implementation-vertx:RELEASE
//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();
        }
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can revert to:

//DEPS io.smallrye:smallrye-graphql-client-implementation-vertx:2.10.0

if you want, but for this version it will not work without:

//DEPS org.eclipse:yasson:2.0.4

And it would be better to have always the latest version in the documentation. Somehow with RELEASE it is ever green.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are even integration tests in the jbang project using the RELEASE notation:

https://github.com/jbangdev/jbang/blob/7af0ad56db2162121ab48b37132008417528a7c0/itests/SankeyPlotTestWithDeps.java#L16-L18

Copy link
Member

@jmartisk jmartisk Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok somehow it works for me now with RELEASE (no idea why it didn't yesterday, perhaps because I had just upgraded JBang and there was some stale data in the caches), but it picks up 2.10.0, so it still needs the dependency... But hey, I think we will release 2.11 (or 2.10.1) within a few weeks, so I guess we could just keep they way you have it now, and hopefully it will start working by itself then

Copy link
Member

@jmartisk jmartisk Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think of it, the documentation will also only deploy when we do a release, so with the next release + documentation deploy, it should just start working

@jmartisk
Copy link
Member

jmartisk commented Sep 25, 2024

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 jmartisk merged commit e5f605f into smallrye:main Sep 26, 2024
4 checks passed
@jmartisk
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants