Open QuintinWillison opened 4 years ago
I've reproduced this in IntelliJ IDEA CE when working with a brand new Gradle project. I have the ably-java Java jar in a libs
folder and introduced it to the project using:
dependencies {
implementation fileTree(include: ['*.jar'], dir: 'libs')
}
It builds fine:
hello-deltas-java % ./gradlew build
BUILD SUCCESSFUL in 638ms
5 actionable tasks: 4 executed, 1 up-to-date
but crashes at runtime:
hello-deltas-java % ./gradlew run
> Task :run FAILED
Ably Client Library: java v.1.2.0
Exception in thread "main" java.lang.NoClassDefFoundError: com/google/gson/JsonElement
at io.ably.lib.rest.Auth.setTokenDetails(Auth.java:928)
at io.ably.lib.rest.Auth.<init>(Auth.java:893)
at io.ably.lib.rest.AblyBase.<init>(AblyBase.java:72)
at io.ably.lib.rest.AblyRest.<init>(AblyRest.java:25)
at io.ably.lib.realtime.AblyRealtime.<init>(AblyRealtime.java:53)
at io.ably.lib.realtime.AblyRealtime.<init>(AblyRealtime.java:44)
at io.ably.examples.deltas.Hello.main(Hello.java:11)
Caused by: java.lang.ClassNotFoundException: com.google.gson.JsonElement
at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
at java.lang.ClassLoader.loadClass(ClassLoader.java:418)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:355)
at java.lang.ClassLoader.loadClass(ClassLoader.java:351)
... 7 more
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':run'.
> Process 'command '/Library/Java/JavaVirtualMachines/jdk1.8.0_241.jdk/Contents/Home/bin/java'' finished with non-zero exit value 1
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. Run with --scan to get full insights.
* Get more help at https://help.gradle.org
BUILD FAILED in 547ms
2 actionable tasks: 1 executed, 1 up-to-date
The code doesn't do anything valuable yet - all I have so far is:
package io.ably.examples.deltas;
import io.ably.lib.BuildConfig;
import io.ably.lib.realtime.AblyRealtime;
import io.ably.lib.types.AblyException;
public class Hello {
public static void main(final String[] args) throws AblyException {
System.out.println("Ably Client Library: " + BuildConfig.NAME + " v." + BuildConfig.VERSION);
final AblyRealtime ably = new AblyRealtime("xxxxx");
}
}
Actually that could be unrelated, being a direct result of me trying to 'manually' bring this in as a jar. I've added GSON dependency but now getting:
java.lang.ClassNotFoundException: org.java_websocket.client.WebSocketClient
But adding it as a dependency in the way that you have isn't going to work because you're not bringing in any of the library dependencies. That's why the README says to reference it via
compile 'io.ably:ably-java:1.1.11'
Otherwise you'd need a fat jar, and we don't publish a fat jar. I don't think what you're seeing is an issue specific to gson.
We bring GSON in as an implementation
dependency, but we expose it as part of our API - see MessageExtras' constructor, for example.
I expect that we either need to change the dependency from implementation
to api
(simple fix) or consider making a breaking change so that Gson APIs like JsonElement
and JsonObject
are not exposed directly via our API (more predictable fix for supportability going forwards).
I expect that we either need to change the dependency from
implementation
toapi
(simple fix)
My understanding is that this change won't work. It will enable our code to compile against the API declared by that dependency, but it won't cause the dependency to be included in any build that includes our library. Then the dependency will be missing at runtime.
consider making a breaking change so that Gson APIs like
JsonElement
andJsonObject
are not exposed directly via our API (more predictable fix for supportability going forwards)
I'm interested to see what that would be.
The ideal thing would be that the platforms (Android and JDK) have a de facto standard JSON serialisability interface, at least, and then we wouldn't need to make that decision ourselves.
Just to add an example of this problem, when developing the Kafka Connect Ably Connector I needed to set message extras which involved importing io.ably.lib.util.JsonUtils.JsonUtilsObject
, and that failed to compile unless I explicitly added the gson dependency. My Java experience is limited though, so I may well have done that incorrectly.
I've encountered this issue in Ably Asset Tracking too. When I was parsing the presence data sent from ably-js
client it turned out that it comes in the JsonObject
format which is a type from the Gson library. Luckily, we're also using the Gson library in the project but there are other popular JSON parsing libraries (like Moshi) that other people might be using in their projects.
If we could somehow remove the Gson dependency from the API it would be awesome.
Where are we with this? @KacperKluka @QuintinWillison
I believe that we've agreed that gson
should not be a part of the API. Now we need to find something that uses only foundational JDK APIs and could replace it. I've found this class but it seems to be Java EE only.
I was imagining that we would move our API from presenting a GSON type to Object
as a POJO (Plain Old Java Object). As is the case for JavaScript, there are logical Java interfaces for all structures/types defined by JSON.
I guess we might be able to achieve that with GSON remaining as the implementation detail. Or, perhaps, we consider moving to something like Jackson which I believe is a little bit more layered - e.g. ObjectMapper.
Oh, and in response to @KacperKluka's:
Now we need to find something that uses only foundational JDK APIs and could replace it
We don't have to restrict ourselves to only foundational JDK APIs for the implementation detail, to be clear. It's what we expose to our API that we're solving here! 😅
I do not know why we need to replace Gson
as it works perfectly and what we use in SDK should not affect end user. I suggest we return our own object instead of the JsonElement list.
I think this is the only place we expose JsonElement but we will need to make sure that is the case. However, we are using JsonObject in few instances as well so we need to decide what to do with them as well.
@qsdigor I've read your message a few times:
I do not know why we need to replace Gson as it works perfectly and what we use in SDK should not affect end user. I suggest we return our own object instead of the JsonElement list. I think this is the only place we expose JsonElement but we will need to make sure that is the case. However, we are using JsonObject in few instances as well so we need to decide what to do with them as well.
But I remain confused as to how it connects or otherwise relates to my previous message describing a POJO approach at front-end (SDK API) and then either GSON or perhaps Jackson at the back-end (implementation detail). Could you be more specific as to the point you're making, please? Or are you, perhaps, just agreeing? 😉
@QuintinWillison I do not see the urge to replace Gson in the back end of the SDK. What we show or use in the front end to the customer is what we need to agree on. So my suggestion is to leave Gson in the back end of SDK and change return types of problematic methods. We can go two ways here:
We've agreed to come back to this discussion when @qsdigor is back from some time away from this project. I believe the best way forward will be to have a synchronous call to get everybody on the same page and plot a route forwards. That call will ideally include @KacperKluka and @ikbalkaya.
Example of customer confusion from GSON / JSONObject: CSS-557 (internal).
Reported by a customer (internal conversation). In their words, given this tripped them up...
Initial Report:
Follow-up opinion as to how this should be instead:
┆Issue is synchronized with this Jira Research by Unito