IllegalAccessError is thrown when Jest tries to access internal deepCopy method com.google.gson.JsonObject class

[sarastinishi]

I am using Jest client to work with Elasticsearch from inside Spark application. While it runs as expected when executing Spark in local mode, it fails in cluster mode with the following exception:

Exception in thread "main" java.lang.IllegalAccessError: tried to access method com.google.gson.JsonObject.deepCopy()Lcom/google/gson/JsonObject; from class com.google.gson.GsonUtils
at com.google.gson.GsonUtils.deepCopy(GsonUtils.java:8)
at io.searchbox.core.SearchResult.extractHit(SearchResult.java:114)
at io.searchbox.core.SearchResult.getHits(SearchResult.java:82)
at io.searchbox.core.SearchResult.getHits(SearchResult.java:63)
at io.searchbox.core.SearchResult.getHits(SearchResult.java:59)

There was a commit cdd62a8 made on Dec 26, 2015 that introduced GsonUtils class that simply calls package level deepCopy method of com.google.gson.JsonObject class by simply defining GsonUtils in com.google.gson package. It seems like a not very good practice to access internal methods. It may cause problems when using multiple classloaders (just like with Spark example above).

[kramer]

See the discussion that lead me to create that utils class here: google/gson#760

[sarastinishi]

Do you think it's a good idea to still call non public methods through defining gson package in Jest? Isn't it better to use some other way to clone objects that does not cause any exceptions?

[sarastinishi]

Thank you for bringing up this issue @yuriyfilonov!
I would very much rather not have another additional dependency just for this purpose. If you have an alternative approach or solution let's discuss that on #351 but unfortunately I won't be merging this PR.

I am confused. You prefer to allow exceptions to one additional dependency?

[kramer]

As you mentioned the issue occurs only under certain environments ("when using multiple classloaders (just like with Spark example above)") which I would guess is not the prominent use case for this library. This decreases the severity of the exception in the big picture. On the other hand you are suggesting to introduce a new dependency which will come with it's own baggage of possible bugs and maintenance needs, which makes it a "load" (or debt if you will) on the whole project.

Considering the above pro/con comparison it makes more sense (at least maintenance-wise) to not take this suggested fix into the project.

And to answer your question, no I don't think the existing utils class is a good solution, it is simply a workaround. I'd be more than happy to replace it with a similarly light weight solution.

[sarastinishi]

Makes sense. I have found another solution to this problem. Please, see pl #353