Change in parsing between versions 2.7 and 2.8.5

[Aladdill]

Hi,
My company used till recently version 2.1, while working on a certain feature, we found, that one of bugs in 2.1 prevent from us creating the feature, so we upgraded to version 2.8.6. after upgrade feature worked well, but one of our POJOs was parsed differently. After some investigation we found, that parsing still works fine in version 2.7. Attached a test that will pass on 2.7 and fail on 2.8.6.
Thanks.

JsonParsingTest.zip

[lyubomyr-shaydariv]

Despite it looks like Gson 2.8.2 seems to have changed the way it interprets numbers, you should not serialize/deserialize classes you don't control. See these for more information: #1573, #1613, #1660.

What you should do is:

private static final Gson gson = new GsonBuilder()
.registerTypeAdapterFactory(new TypeAdapterFactory() {
	@Override
	@Nullable
	public <T> TypeAdapter<T> create(final Gson gson, final TypeToken<T> typeToken) {
		if ( !Range.class.isAssignableFrom(typeToken.getRawType()) ) {
			return null;
		}
		final Type typeArgument = ((ParameterizedType) typeToken.getType()).getActualTypeArguments()[0];
		@SuppressWarnings("unchecked")
		final TypeAdapter<Comparable<?>> elementTypeAdapter = (TypeAdapter<Comparable<?>>) gson.getDelegateAdapter(this, TypeToken.get(typeArgument));
		final TypeAdapter<Range<? extends Comparable<?>>> rangeTypeAdapter = new TypeAdapter<Range<? extends Comparable<?>>>() {
			@Override
			public void write(final JsonWriter jsonWriter, final Range<? extends Comparable<?>> range)
					throws IOException {
				// TODO add from/to support?
				jsonWriter.beginObject();
				jsonWriter.name("v1");
				elementTypeAdapter.write(jsonWriter, range.v1);
				jsonWriter.name("v2");
				elementTypeAdapter.write(jsonWriter, range.v2);
				jsonWriter.endObject();
			}

			@Override
			public Range<? extends Comparable<?>> read(final JsonReader jsonReader)
					throws IOException {
				// TODO add from/to support?
				jsonReader.beginObject();
				@Nullable
				Comparable<?> v1 = null;
				@Nullable
				Comparable<?> v2 = null;
				while ( jsonReader.hasNext() ) {
					switch ( jsonReader.nextName() ) {
					case "v1":
						v1 = elementTypeAdapter.read(jsonReader);
						break;
					case "v2":
						v2 = elementTypeAdapter.read(jsonReader);
						break;
					default:
						// ignore the rest
						jsonReader.skipValue();
						break;
					}
				}
				if ( v1 == null || v2 == null ) {
					throw new IllegalArgumentException("Insufficient range: " + v1 + ", " + v2);
				}
				@SuppressWarnings({ "unchecked", "rawtypes" })
				final Range<? extends Comparable<?>> range = new Range(v1, v2);
				return range;
			}
		}
				.nullSafe();
		@SuppressWarnings("unchecked")
		final TypeAdapter<T> castRangeTypeAdapter = (TypeAdapter<T>) rangeTypeAdapter;
		return castRangeTypeAdapter;
	}
})
.create();

The type adapter will pass the following test:

final JsonElement el = JsonParser.parseString("{\"from\":1,\"to\":50,\"v1\":1,\"v2\":50}");
System.out.println("el: " + el);
final Range<Integer> range = gson.fromJson(el, rangeOfIntegerType);
System.out.println("Range: " + range);
Assertions.assertEquals(new Range<>(1, 50), range);
System.out.println(gson.toJson(new Range<>(1, 50), rangeOfIntegerType));

[Aladdill]

Thanks, I thought of creating type adapter.

[sindilevich]

Great insight into the internal workings of Gson, @lyubomyr-shaydariv , thank you!

However, what is the reason for a TypeAdapter for org.jooq.lambda.tuple.Range<T>, which is a simple structure of two T v1, T v2 fields? This class neither more complex than any other POJO I control, nor it requires special handling for (de-)serialization.
A simple bi-directional conversions of an instance of new Range<Integer>(1, 50) <=> {v1: 1, v2: 50}, that's all. And it seemed working properly up until Gson 2.8.2, as you've mentioned.

Requiring a TypeAdapter for each third-party library class that only requires a straightforward bi-directional conversion is an overkill, and will introduce tons of boilerplate code into projects.

[lyubomyr-shaydariv]

@sindilevich

However, what is the reason for a TypeAdapter for org.jooq.lambda.tuple.Range, which is a simple structure of two T v1, T v2 fields? This class neither more complex than any other POJO I control, nor it requires special handling for (de-)serialization.

Well, I wouldn't call that class a POJO at all... Gson relies on non-transient/non-excluded instance fields regardless their visibility in respective plain objects. What would happen if the library vendor decides adding new (especially private) fields or removing existing (especially private) ones? Your JSON-serialized would get broken just because you don't control the structure of the classes. For another example, the from and to properties from the JSON above might make me think that they might appear somewhere in the third-party classes, but at least I don't see these two in org.jooq:jool:0.9.12. I just find such classes too fragile to use in such a scenario.

And it seemed working properly up until Gson 2.8.2, as you've mentioned.

I don't know what really caused the change, but for some reason Gson does not seem to be able to infer the second type parameter:

• The actual type is declared as Range<Integer>, and it's base type is Tuple2<T, T>
• When deserializing, Gson considers the first parameter as Integer (v1 works fine for your test), but considers the second parameter as type parameter T (not Integer), I guess, falling back to an Object/Double: something like Tuple2<Integer, T> (if I understood it). Therefore v2 does not pass your test as Gson does not handle numeric types as expected (see more at Support arbitrary Number implementation for Object and Number deserialization #1290 that's loosely related to your case, and

  gson/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java

  Line 160 in ceae88b

  Type fieldType = $Gson$Types.resolve(type.getType(), raw, field.getGenericType());

  to examine the actual type arguments)).

This issue looks like a tricky found regression happened, I'm assuming, somewhere between 2.8.1 and 2.8.2.

Requiring a TypeAdapter for each third-party library class that only requires a straightforward bi-directional conversion is an overkill, and will introduce tons of boilerplate code into projects.

You don't need type adapters (that might be implemented reusable anyway) if you have plain DTOs that serve the dumb (de)serialization purposes (no direct third-party coupling but controlling the entire binding structure, no inheritance, no complicated implementation interface model, etc):

@Test
public void testDedicatedDto()
		throws IOException {
	final Gson gson = new GsonBuilder()
			// no adapter
			.create();
	final Type rangeDtoOfIntegerType = new TypeToken<RangeDto<Integer>>() {}.getType();
	try ( final JsonReader jsonReader = new JsonReader(new StringReader(JSON)) ) {
		final Range<Integer> range = gson.<RangeDto<Integer>>fromJson(jsonReader, rangeDtoOfIntegerType)
				.toRange();
		Assertions.assertEquals(new Range<>(1, 50), range);
	}
}

private static final class RangeDto<T extends Comparable<T>> {

	final T v1;
	final T v2;

	private RangeDto(final T v1, final T v2) {
		this.v1 = v1;
		this.v2 = v2;
	}

	Range<T> toRange() {
		return new Range<>(v1, v2);
	}

}

The class above declared exactly two fields to serialize and deserialize, so you'd not care if the Range class changes its internal structure. #1573 and #1613 are great examples what would happen when handling uncontrolled types.

[sindilevich]

@lyubomyr-shaydariv, thank you for your detailed answer!

I would like to clarify a couple of points raising from your answer, however.

Your JSON-serialized would get broken just because you don't control the structure of the classes.

I fully agree with this statement, yet please acknowledge that a TypeAdapter/DTO would not help out in this scenario either. Whether a third-party class adds new fields, removes or changes the existing ones, you'd need to update your TypeAdapter/DTO. There simply isn't a bulletproof solution here. Moreover, if you need that third-party in your domain, and reach as far as introducing your own POJO for (de-)serialization, you would just as busy updating the POJO and the bi-directional conversion process, as if you were using the third-party's class straight away for (de-)serialization.

That's why I see no reason introducing the JSON <-> RangeDto<T> <-> Range<T> conversion chain in place for JSON <-> Range<T>.

This issue looks like a tricky found regression happened, I'm assuming, somewhere between 2.8.1 and 2.8.2.

May I assume the Gson maintenance team will consider this issue as a bug, therefore investigate the issue and possibly fix it in further versions of the library?

Thank you for your time!

[lyubomyr-shaydariv]

Whether a third-party class adds new fields, removes or changes the existing ones, you'd need to update your TypeAdapter/DTO. There simply isn't a bulletproof solution here. Moreover, if you need that third-party in your domain, and reach as far as introducing your own POJO for (de-)serialization, you would just as busy updating the POJO and the bi-directional conversion process, as if you were using the third-party's class straight away for (de-)serialization.

Yes, there is not a bulletproof solution, but I believe it's a small price for not having issues that might be expensive in the future. Especially, if there's a regression in newer versions of Gson. :)

May I assume the Gson maintenance team will consider this issue as a bug, therefore investigate the issue and possibly fix it in further versions of the library?

Sure, why not? It really looks like a regression/bug in how Gson resolves types and type parameters. I just don't know how long it would take to get it fixed/reviewed/merged/released if anyone picks it and gets it fixed.

[Marcono1234]

I fully agree with this statement, yet please acknowledge that a TypeAdapter/DTO would not help out in this scenario either. Whether a third-party class adds new fields, removes or changes the existing ones, you'd need to update your TypeAdapter/DTO.

@sindilevich, the TypeAdapter @lyubomyr-shaydariv proposed above only works with the public API Range exposes. So unless the public API changes (which is very unlikely and would then break more than serialization) it will continue to work, even if the internal implementation, such as the private fields, changes in any way.
The strings "v1" and "v2" only refer to the names of the JSON properties. However, since the TypeAdapter handles writing and reading of it, it can choose any name for them. It could for example also call them "from" and "to" (as hinted by the comment in the example code). Or it could even (de-)serialize the Range as JSON array, e.g.: new Range(1, 3) -Gson-> [1, 3].

[Marcono1234]

It appears #1391 fixes the issue described here.

[sindilevich]

@Marcono1234, thank you so much for pointing to the #1391 fix! Will monitor that to see it really resolves the issue with org.jooq.lambda.tuple.Range<T> as well. Any idea when the fix is merged/released?

[Marcono1234]

Sorry, I am not a maintainer of this project, so I don't know when this will be merged.

[Marcono1234]

It looks like at least the originally provided test cases succeeds in Gson 2.9.1 with org.jooq:jool:0.9.14 as dependency for the Range class. Please create a new issue in case you still experience a similar problem with the latest Gson versions.