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

Usage of undocumented Gson.getDelegateAdapter(null, ...) #129

Closed
Marcono1234 opened this issue Aug 23, 2023 · 3 comments
Closed

Usage of undocumented Gson.getDelegateAdapter(null, ...) #129

Marcono1234 opened this issue Aug 23, 2023 · 3 comments

Comments

@Marcono1234
Copy link

Hello,
your code uses Gson.getDelegateAdapter(null, ...):
https://github.com/vertigo-io/vertigo-extensions/blob/b0f90b5e1ef1c9649e7c0b463b316aa7220f1a15/vertigo-vega/src/main/java/io/vertigo/vega/engines/webservice/json/GoogleJsonEngine.java#L310

Using null as argument relies on undocumented Gson implementation details (which also haven't been consistent over the years). In upcoming Gson versions null will be rejected as argument (see also google/gson#2435 (comment)).

You can probably rewrite your code to use a custom Gson TypeAdapterFactory instead to achieve the same functionality. If you need help with that, feel free to let me know.

@mlaroche
Copy link
Contributor

Hello, thanks for the head's up about that upcoming change. Really appreciate it.
Good to know that Gson is still in active developpement because we appreciate its lightweight philosophy over Jackson.
We knew that we were relying on a non plublicly documented behaviour, so that we should have to replace it sooner or later.
If fact it was a 'shorcut' to take advantage of default deserialization based on reflection in gson.
I think the safe way to deal with this it to perform manually the deserialization of each property of the object in the existing TypeAdapter. That way we can also apply filtering of the data that's coming into our classes.

What would be the benefit of switching to a TypeAdapterFactory? Performance wise, the TypeAdapter will be efficient because reflection calls are cached with our underlying implementation.

Thanks a lot for your hints.

Have a good day

@Marcono1234
Copy link
Author

What would be the benefit of switching to a TypeAdapterFactory?

Mainly that it 'officially' supports the use case of looking up a delegate adapter. That delegate will be any other custom adapter you have registered for that type, or otherwise one of the built-in adapters, such as the reflection-based one.

The documentation also has some examples for this, but basically your implementation of the TypeAdapterFactory#create method would call Gson.getDelegateAdapter(this, type) to obtain the delegate, and optionally use Gson.getAdapter(...) to obtain adapters for any other types you might need. If you still want to operate on JsonObject, one of those adapters could for example be the TypeAdapter<JsonObject>, so that you can read the JsonObject from the JsonReader.

In general using TypeAdapter will probably give you better performance because it directly operates on the underlying streams whereas JsonSerializer / JsonDeserializer first have to create a full representation of the JSON data as JsonElement tree before it is written / read.

mlaroche added a commit that referenced this issue Aug 23, 2023
@mlaroche
Copy link
Contributor

Thank you very much for your answer and all the explanations.
I've commited a new version with the TypeAdapter that doesn't rely on the delegate adapter. Staying with the TypeAdapter approche helps us minimize the number of Gson API we consume and therefore makes the maintenance easier.

Have a nice day, and thanks again

PS :
Despite the small number of stars on the repo, vertigo is used by a good amount of closed source projects, so they will eventually all thank you for your warning !

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

No branches or pull requests

2 participants