Description
Problem solved by the feature
Avoid recursive null-safe wrapping.
Feature description
Currently the TypeAdapter.nullSafe() method is implemented like this:
public final TypeAdapter<T> nullSafe() {
return new TypeAdapter<T>() {
@Override
public void write(JsonWriter out, T value) throws IOException {
if (value == null) {
out.nullValue();
} else {
TypeAdapter.this.write(out, value);
}
}
@Override
public T read(JsonReader reader) throws IOException {
if (reader.peek() == JsonToken.NULL) {
reader.nextNull();
return null;
}
return TypeAdapter.this.read(reader);
}
};
}As seen, the TypeAdapter.nullSafe method is not guarded with its own type check, thus invoking the TypeAdapter.nullSafe method multiple times wraps the type adapter in the null-safe wrapper multiple times.
Currently it passes the following unit test(s):
@ParameterizedTest
@ValueSource(ints = { 0, 1, 10 })
public void testNullSafe0OrMore(final int expectedSafeCount) {
final TypeAdapter<Object> nullSafeTypeAdapter = failingTypeAdapter(expectedSafeCount);
final RuntimeException ex = Assertions.assertThrows(RuntimeException.class, () -> nullSafeTypeAdapter.toJson(dummy));
// probably bad check (is StackTraceElement.toString() guaranteed to return full.method.Name)
final long actualNullSafeCount = Stream.of(ex.getStackTrace())
.map(StackTraceElement::toString)
.filter(toString -> toString.startsWith("com.google.gson.TypeAdapter$1.write("))
.count();
Assertions.assertEquals(expectedSafeCount, actualNullSafeCount);
}
private static final Object dummy = new Object();
private static TypeAdapter<Object> failingTypeAdapter(final int nullSafeCount) {
if ( nullSafeCount < 0 ) {
throw new IllegalArgumentException();
}
TypeAdapter<Object> typeAdapter = new TypeAdapter<>() {
@Override public void write(final JsonWriter out, final Object value) { throw new RuntimeException("nullSafe count: " + nullSafeCount); }
@Override public Object read(final JsonReader in) { throw new AssertionError("must never be invoked"); }
};
for ( int i = 0; i < nullSafeCount; i++ ) {
typeAdapter = typeAdapter.nullSafe();
}
return typeAdapter;
}Alternatives / workarounds
The null-safe type adapter class would probably be extracted as a private named class, say NullSafeTypeAdapter. Then the null-safe wrapper method would be able to check if the given type adapter instanceofs against the NullSafeTypeAdapter, and if it's true simply return it as it is already wrapped in the null-safe type adapter. Otherwise the given type adapter instance would be wrapped in the NullSafeTypeAdapter class, thus once passing the following assertion:
Assertions.assertEquals(expectedSafeCount > 0 ? 1 : 0, actualNullSafeCount);This wouldn't probably be any significant improvement as the use site in most code, I believe, is controlled and uses TypeAdapter.nullSafe() once, so there are no recursive calls as it suggested with expectedSafeCount in the unit test above. But it would be probably be useful for more advanced uses of TypeAdapter (thus its chaining) if any. This change would be also backwards-compatible.