Fix TypeAdapterRuntimeTypeWrapper not detecting reflective TreeTypeAdapter and FutureTypeAdapter (#1787)

* Fix TypeAdapterRuntimeTypeWrapper not detecting reflective TreeTypeAdapter

Previously on serialization TypeAdapterRuntimeTypeWrapper preferred a
TreeTypeAdapter without `serializer` which falls back to the reflective
adapter. This behavior was incorrect because it caused the reflective
adapter for a Base class to be used for serialization (indirectly as
TreeTypeAdapter delegate) instead of using the reflective adapter for
a Subclass extending Base.

* Address review feedback

* Convert TypeAdapterRuntimeTypeWrapperTest to JUnit 4 test

* Prefer wrapped reflective adapter for serialization of subclass

* Detect reflective adapter used as delegate for Gson.FutureTypeAdapter

* Tiny style tweak.

Co-authored-by: Éamonn McManus <emcmanus@google.com>

gson/src/main/java/com/google/gson/Gson.java

@@ -32,6 +32,7 @@ import com.google.gson.internal.bind.MapTypeAdapterFactory;
 import com.google.gson.internal.bind.NumberTypeAdapter;
 import com.google.gson.internal.bind.ObjectTypeAdapter;
 import com.google.gson.internal.bind.ReflectiveTypeAdapterFactory;
+import com.google.gson.internal.bind.SerializationDelegatingTypeAdapter;
 import com.google.gson.internal.bind.TypeAdapters;
 import com.google.gson.internal.sql.SqlTypesSupport;
 import com.google.gson.reflect.TypeToken;
@@ -1315,7 +1316,7 @@ public final class Gson {
     return fromJson(new JsonTreeReader(json), typeOfT);
   }
 
-  static class FutureTypeAdapter<T> extends TypeAdapter<T> {
+  static class FutureTypeAdapter<T> extends SerializationDelegatingTypeAdapter<T> {
     private TypeAdapter<T> delegate;
 
     public void setDelegate(TypeAdapter<T> typeAdapter) {
@@ -1325,18 +1326,23 @@ public final class Gson {
       delegate = typeAdapter;
     }
 
-    @Override public T read(JsonReader in) throws IOException {
+    private TypeAdapter<T> delegate() {
       if (delegate == null) {
-        throw new IllegalStateException();
+        throw new IllegalStateException("Delegate has not been set yet");
       }
-      return delegate.read(in);
+      return delegate;
+    }
+
+    @Override public TypeAdapter<T> getSerializationDelegate() {
+      return delegate();
+    }
+
+    @Override public T read(JsonReader in) throws IOException {
+      return delegate().read(in);
     }
 
     @Override public void write(JsonWriter out, T value) throws IOException {
-      if (delegate == null) {
-        throw new IllegalStateException();
-      }
-      delegate.write(out, value);
+      delegate().write(out, value);
     }
   }
 

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

@@ -0,0 +1,14 @@
+package com.google.gson.internal.bind;
+
+import com.google.gson.TypeAdapter;
+
+/**
+ * Type adapter which might delegate serialization to another adapter.
+ */
+public abstract class SerializationDelegatingTypeAdapter<T> extends TypeAdapter<T> {
+  /**
+   * Returns the adapter used for serialization, might be {@code this} or another adapter.
+   * That other adapter might itself also be a {@code SerializationDelegatingTypeAdapter}.
+   */
+  public abstract TypeAdapter<T> getSerializationDelegate();
+}

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

@@ -38,7 +38,7 @@ import java.lang.reflect.Type;
  * tree adapter may be serialization-only or deserialization-only, this class
  * has a facility to lookup a delegate type adapter on demand.
  */
-public final class TreeTypeAdapter<T> extends TypeAdapter<T> {
+public final class TreeTypeAdapter<T> extends SerializationDelegatingTypeAdapter<T> {
   private final JsonSerializer<T> serializer;
   private final JsonDeserializer<T> deserializer;
   final Gson gson;
@@ -97,6 +97,15 @@ public final class TreeTypeAdapter<T> extends TypeAdapter<T> {
         : (delegate = gson.getDelegateAdapter(skipPast, typeToken));
   }
 
+  /**
+   * Returns the type adapter which is used for serialization. Returns {@code this}
+   * if this {@code TreeTypeAdapter} has a {@link #serializer}; otherwise returns
+   * the delegate.
+   */
+  @Override public TypeAdapter<T> getSerializationDelegate() {
+    return serializer != null ? this : delegate();
+  }
+
   /**
    * Returns a new factory that will match each type against {@code exactType}.
    */
@@ -169,5 +178,5 @@ public final class TreeTypeAdapter<T> extends TypeAdapter<T> {
     @Override public <R> R deserialize(JsonElement json, Type typeOfT) throws JsonParseException {
       return (R) gson.fromJson(json, typeOfT);
     }
-  };
+  }
 }

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

@@ -53,10 +53,12 @@ final class TypeAdapterRuntimeTypeWrapper<T> extends TypeAdapter<T> {
     if (runtimeType != type) {
       @SuppressWarnings("unchecked")
       TypeAdapter<T> runtimeTypeAdapter = (TypeAdapter<T>) context.getAdapter(TypeToken.get(runtimeType));
+      // For backward compatibility only check ReflectiveTypeAdapterFactory.Adapter here but not any other
+      // wrapping adapters, see https://github.com/google/gson/pull/1787#issuecomment-1222175189
       if (!(runtimeTypeAdapter instanceof ReflectiveTypeAdapterFactory.Adapter)) {
         // The user registered a type adapter for the runtime type, so we will use that
         chosen = runtimeTypeAdapter;
-      } else if (!(delegate instanceof ReflectiveTypeAdapterFactory.Adapter)) {
+      } else if (!isReflective(delegate)) {
         // The user registered a type adapter for Base class, so we prefer it over the
         // reflective type adapter for the runtime type
         chosen = delegate;
@@ -68,12 +70,30 @@ final class TypeAdapterRuntimeTypeWrapper<T> extends TypeAdapter<T> {
     chosen.write(out, value);
   }
 
+  /**
+   * Returns whether the type adapter uses reflection.
+   *
+   * @param typeAdapter the type adapter to check.
+   */
+  private static boolean isReflective(TypeAdapter<?> typeAdapter) {
+    // Run this in loop in case multiple delegating adapters are nested
+    while (typeAdapter instanceof SerializationDelegatingTypeAdapter) {
+      TypeAdapter<?> delegate = ((SerializationDelegatingTypeAdapter<?>) typeAdapter).getSerializationDelegate();
+      // Break if adapter does not delegate serialization
+      if (delegate == typeAdapter) {
+        break;
+      }
+      typeAdapter = delegate;
+    }
+
+    return typeAdapter instanceof ReflectiveTypeAdapterFactory.Adapter;
+  }
+
   /**
    * Finds a compatible runtime type if it is more specific
    */
-  private Type getRuntimeTypeIfMoreSpecific(Type type, Object value) {
-    if (value != null
-        && (type == Object.class || type instanceof TypeVariable<?> || type instanceof Class<?>)) {
+  private static Type getRuntimeTypeIfMoreSpecific(Type type, Object value) {
+    if (value != null && (type instanceof Class<?> || type instanceof TypeVariable<?>)) {
       type = value.getClass();
     }
     return type;

gson/src/test/java/com/google/gson/functional/TypeAdapterRuntimeTypeWrapperTest.java

@@ -0,0 +1,193 @@
+package com.google.gson.functional;
+
+import static org.junit.Assert.assertEquals;
+
+import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.JsonDeserializationContext;
+import com.google.gson.JsonDeserializer;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonPrimitive;
+import com.google.gson.JsonSerializationContext;
+import com.google.gson.JsonSerializer;
+import com.google.gson.TypeAdapter;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonWriter;
+import java.io.IOException;
+import java.lang.reflect.Type;
+import org.junit.Test;
+
+public class TypeAdapterRuntimeTypeWrapperTest {
+  private static class Base {
+  }
+  private static class Subclass extends Base {
+    @SuppressWarnings("unused")
+    String f = "test";
+  }
+  private static class Container {
+    @SuppressWarnings("unused")
+    Base b = new Subclass();
+  }
+  private static class Deserializer implements JsonDeserializer<Base> {
+    @Override
+    public Base deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) {
+      throw new AssertionError("not needed for this test");
+    }
+  }
+
+  /**
+   * When custom {@link JsonSerializer} is registered for Base should
+   * prefer that over reflective adapter for Subclass for serialization.
+   */
+  @Test
+  public void testJsonSerializer() {
+    Gson gson = new GsonBuilder()
+      .registerTypeAdapter(Base.class, new JsonSerializer<Base>() {
+        @Override
+        public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) {
+          return new JsonPrimitive("serializer");
+        }
+      })
+      .create();
+
+    String json = gson.toJson(new Container());
+    assertEquals("{\"b\":\"serializer\"}", json);
+  }
+
+  /**
+   * When only {@link JsonDeserializer} is registered for Base, then on
+   * serialization should prefer reflective adapter for Subclass since
+   * Base would use reflective adapter as delegate.
+   */
+  @Test
+  public void testJsonDeserializer_ReflectiveSerializerDelegate() {
+    Gson gson = new GsonBuilder()
+      .registerTypeAdapter(Base.class, new Deserializer())
+      .create();
+
+    String json = gson.toJson(new Container());
+    assertEquals("{\"b\":{\"f\":\"test\"}}", json);
+  }
+
+  /**
+   * When {@link JsonDeserializer} with custom adapter as delegate is
+   * registered for Base, then on serialization should prefer custom adapter
+   * delegate for Base over reflective adapter for Subclass.
+   */
+  @Test
+  public void testJsonDeserializer_CustomSerializerDelegate() {
+    Gson gson = new GsonBuilder()
+      // Register custom delegate
+      .registerTypeAdapter(Base.class, new TypeAdapter<Base>() {
+        @Override
+        public Base read(JsonReader in) throws IOException {
+          throw new UnsupportedOperationException();
+        }
+        @Override
+        public void write(JsonWriter out, Base value) throws IOException {
+          out.value("custom delegate");
+        }
+      })
+      .registerTypeAdapter(Base.class, new Deserializer())
+      .create();
+
+    String json = gson.toJson(new Container());
+    assertEquals("{\"b\":\"custom delegate\"}", json);
+  }
+
+  /**
+   * When two (or more) {@link JsonDeserializer}s are registered for Base
+   * which eventually fall back to reflective adapter as delegate, then on
+   * serialization should prefer reflective adapter for Subclass.
+   */
+  @Test
+  public void testJsonDeserializer_ReflectiveTreeSerializerDelegate() {
+    Gson gson = new GsonBuilder()
+      // Register delegate which itself falls back to reflective serialization
+      .registerTypeAdapter(Base.class, new Deserializer())
+      .registerTypeAdapter(Base.class, new Deserializer())
+      .create();
+
+    String json = gson.toJson(new Container());
+    assertEquals("{\"b\":{\"f\":\"test\"}}", json);
+  }
+
+  /**
+   * When {@link JsonDeserializer} with {@link JsonSerializer} as delegate
+   * is registered for Base, then on serialization should prefer
+   * {@code JsonSerializer} over reflective adapter for Subclass.
+   */
+  @Test
+  public void testJsonDeserializer_JsonSerializerDelegate() {
+    Gson gson = new GsonBuilder()
+      // Register JsonSerializer as delegate
+      .registerTypeAdapter(Base.class, new JsonSerializer<Base>() {
+        @Override
+        public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) {
+          return new JsonPrimitive("custom delegate");
+        }
+      })
+      .registerTypeAdapter(Base.class, new Deserializer())
+      .create();
+
+    String json = gson.toJson(new Container());
+    assertEquals("{\"b\":\"custom delegate\"}", json);
+  }
+
+  /**
+   * When a {@link JsonDeserializer} is registered for Subclass, and a custom
+   * {@link JsonSerializer} is registered for Base, then Gson should prefer
+   * the reflective adapter for Subclass for backward compatibility (see
+   * https://github.com/google/gson/pull/1787#issuecomment-1222175189) even
+   * though normally TypeAdapterRuntimeTypeWrapper should prefer the custom
+   * serializer for Base.
+   */
+  @Test
+  public void testJsonDeserializer_SubclassBackwardCompatibility() {
+    Gson gson = new GsonBuilder()
+      .registerTypeAdapter(Subclass.class, new JsonDeserializer<Subclass>() {
+        @Override
+        public Subclass deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) {
+          throw new AssertionError("not needed for this test");
+        }
+      })
+      .registerTypeAdapter(Base.class, new JsonSerializer<Base>() {
+        @Override
+        public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) {
+          return new JsonPrimitive("base");
+        }
+      })
+      .create();
+
+    String json = gson.toJson(new Container());
+    assertEquals("{\"b\":{\"f\":\"test\"}}", json);
+  }
+
+  private static class CyclicBase {
+    @SuppressWarnings("unused")
+    CyclicBase f;
+  }
+
+  private static class CyclicSub extends CyclicBase {
+    @SuppressWarnings("unused")
+    int i;
+
+    public CyclicSub(int i) {
+      this.i = i;
+    }
+  }
+
+  /**
+   * Tests behavior when the type of a field refers to a type whose adapter is
+   * currently in the process of being created. For these cases {@link Gson}
+   * uses a future adapter for the type. That adapter later uses the actual
+   * adapter as delegate.
+   */
+  @Test
+  public void testGsonFutureAdapter() {
+    CyclicBase b = new CyclicBase();
+    b.f = new CyclicSub(2);
+    String json = new Gson().toJson(b);
+    assertEquals("{\"f\":{\"i\":2}}", json);
+  }
+}