google / gson

A Java serialization/deserialization library to convert Java Objects into JSON and back
Apache License 2.0
23.13k stars 4.27k forks source link

TypeAdapterFactory INTEGER_FACTORY will change INTEGER type to LONG type #2680

Open L-Ymeng opened 1 month ago

L-Ymeng commented 1 month ago

Gson version

2.10.1

Java / Android version

21

Description

I try to get a JsonElement by toJsonTree method. but i found that when i use integer object as input, i will get a long object with method JsonElement.getAsNumber()

I think this commit caused this problem:https://github.com/google/gson/commit/3e3266cf48f132928225e1561a6ae4cb5503d08f

Is this a bug or a feature?

Expected behavior

use integer type as input, get integer type as output

Actual behavior

use integer type as input, get long type as output

Reproduction steps

    @Test
    public void test(){
        Integer testNum = 1;
        JsonElement jsonTree = new Gson().toJsonTree(testNum);
        System.out.println(jsonTree.getAsNumber().getClass());
    }
Marcono1234 commented 1 month ago

Yes can confirm this; it looks like this was indeed introduced by 3e3266cf48f132928225e1561a6ae4cb5503d08f (by accident): There is only JsonWriter.value(long), so the adapters for all other integral primitive types (byte, short, int) actually call value(long).

Sorry for that. Maybe it would make sense to (partially) revert that commit.

It seems AtomicInteger is affected as well, but it's adapter always behaved this way, that a Long instead of the AtomicInteger is stored in the JsonPrimitive. However, this might be a good thing since it is mutable and might cause confusing behavior if it was stored in JsonPrimitive.

Marcono1234 commented 1 month ago

Ok, so the potential issues with the current implementation are:

Reverting 3e3266cf48f132928225e1561a6ae4cb5503d08f (or at least parts of it) would also revert narrowing numeric conversion, e.g. 1.5f -> (byte) 1, see #2156. Though that was originally reported by me, and I am not sure if / how many users were actually negatively affected by that behavior. That could be solved again by performing the narrowing conversion, but then boxing as Number again and then call JsonWriter#value(Number) instead of JsonWriter#value(long), but performance-wise this will likely not be ideal.

Maybe JsonWriter#value(Number) could also be refactored to first check using a new isTrustedIntegralNumberType, to avoid redundant string.equals("-Infinity") checks in these cases. Though it would have to be verified if that actually makes a noticeable performance difference.

So am I currently not sure what the best approach here would be. @eamonnmcmanus, what do you think?