Multiple invocation of the TypeAdapter.nullSafe method causes the given type adapter to be wrapped recursively.

[lyubomyr-shaydariv]

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.

[Marcono1234]

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.

At least to me that sounds reasonable, thanks for this suggestion! Would you like to implement it?
Might also be a good opportunity to override NullSafeTypeAdapter#toString and return for example "NullSafeTypeAdapter[" + delegate + "]".

Regarding the unit test, maybe it would be easiest to check something like this:

TypeAdapter<?> adapter = ...;
TypeAdapter<?> nullSafeAdapter = adapter.nullSafe();
assertThat(nullSafeAdapter.nullSafe()).isSameInstanceAs(nullSafeAdapter);