Fix #1028: Gson.getDelegateAdapter not working correctly for @JsonAdapter field

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

@@ -468,6 +468,7 @@ public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) {
 
       if (requiresThreadLocalCleanup) {
         calls.remove();
+        jsonAdapterFactory.clearCache();
       }
     }
   }
@@ -523,26 +524,36 @@ public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) {
    * @since 2.2
    */
   public <T> TypeAdapter<T> getDelegateAdapter(TypeAdapterFactory skipPast, TypeToken<T> type) {
-    // Hack. If the skipPast factory isn't registered, assume the factory is being requested via
-    // our @JsonAdapter annotation.
-    if (!factories.contains(skipPast)) {
-      skipPast = jsonAdapterFactory;
-    }
-
-    boolean skipPastFound = false;
-    for (TypeAdapterFactory factory : factories) {
-      if (!skipPastFound) {
-        if (factory == skipPast) {
+    if (skipPast != null) {
+      boolean skipPastFound = false;
+
+      // Hack. If the skipPast factory isn't registered, assume the factory is being requested via
+      // our @JsonAdapter annotation.
+      if (!factories.contains(skipPast)) {
+        if (jsonAdapterFactory.isFieldAdapterFactory(skipPast)) {
+          // @JsonAdapter on field has highest precedence, so if delegate
+          // for it should be searched, consider all factories
           skipPastFound = true;
+        } else {
+          skipPast = jsonAdapterFactory;
         }
-        continue;
       }
 
-      TypeAdapter<T> candidate = factory.create(this, type);
-      if (candidate != null) {
-        return candidate;
+      for (TypeAdapterFactory factory : factories) {
+        if (!skipPastFound) {
+          if (factory == skipPast) {
+            skipPastFound = true;
+          }
+          continue;
+        }
+
+        TypeAdapter<T> candidate = factory.create(this, type);
+        if (candidate != null) {
+          return candidate;
+        }
       }
     }
+
     throw new IllegalArgumentException("GSON cannot serialize " + type);
   }
 

gson/src/main/java/com/google/gson/internal/Excluder.java

@@ -221,16 +221,16 @@ private boolean excludeClassInStrategy(Class<?> clazz, boolean serialize) {
       return false;
   }
 
-  private boolean isAnonymousOrLocal(Class<?> clazz) {
+  private static boolean isAnonymousOrLocal(Class<?> clazz) {
     return !Enum.class.isAssignableFrom(clazz)
         && (clazz.isAnonymousClass() || clazz.isLocalClass());
   }
 
-  private boolean isInnerClass(Class<?> clazz) {
+  private static boolean isInnerClass(Class<?> clazz) {
     return clazz.isMemberClass() && !isStatic(clazz);
   }
 
-  private boolean isStatic(Class<?> clazz) {
+  private static boolean isStatic(Class<?> clazz) {
     return (clazz.getModifiers() & Modifier.STATIC) != 0;
   }
 

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

@@ -16,6 +16,9 @@
 
 package com.google.gson.internal.bind;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import com.google.gson.Gson;
 import com.google.gson.JsonDeserializer;
 import com.google.gson.JsonSerializer;
@@ -32,6 +35,32 @@
  * @since 2.3
  */
 public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapterFactory {
+  private static class DummyTypeAdapterFactory implements TypeAdapterFactory {
+    @Override public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
+      throw new AssertionError("Factory should not be used");
+    }
+  }
+  /**
+   * Factory used for {@link TreeTypeAdapter}s created for a {@code @JsonAdapter}
+   * on a class.
+   */
+  private static final TypeAdapterFactory DUMMY_FACTORY_CLASS = new DummyTypeAdapterFactory();
+  /**
+   * Factory used for {@link TreeTypeAdapter}s created for a {@code @JsonAdapter}
+   * on a field.
+   */
+  private static final TypeAdapterFactory DUMMY_FACTORY_FIELD = new DummyTypeAdapterFactory();
+
+  /**
+   * Cache, mapping from a {@link TypeAdapterFactory} class to a corresponding
+   * instance which was created by {@code this}. Only contains type adapter
+   * factories specified for annotated fields. Allows reusing instances if an
+   * annotation on another field specifies the same adapter factory.
+   *
+   * <p>Does not impose type restrictions on key type to not require casting.
+   */
+  private final ThreadLocal<Map<Class<?>, Object>> fieldAdapterFactoryCache = new ThreadLocal<Map<Class<?>, Object>>();
+
   private final ConstructorConstructor constructorConstructor;
 
   public JsonAdapterAnnotationTypeAdapterFactory(ConstructorConstructor constructorConstructor) {
@@ -46,13 +75,56 @@ public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> targetType) {
     if (annotation == null) {
       return null;
     }
-    return (TypeAdapter<T>) getTypeAdapter(constructorConstructor, gson, targetType, annotation);
+    return (TypeAdapter<T>) getTypeAdapter(constructorConstructor, gson, targetType, annotation, false);
+  }
+
+  /**
+   * Returns whether the non-{@code null} adapter factory was created by
+   * this instance for an annotated field.
+   */
+  public boolean isFieldAdapterFactory(TypeAdapterFactory adapterFactory) {
+    if (adapterFactory == DUMMY_FACTORY_FIELD) {
+      return true;
+    } else if (adapterFactory == DUMMY_FACTORY_CLASS) {
+      return false;
+    }
+
+    Map<Class<?>, Object> cacheMap = fieldAdapterFactoryCache.get();
+    if (cacheMap == null) {
+      // ThreadLocal was initialized by get(), have to remove entry again
+      fieldAdapterFactoryCache.remove();
+      return false;
+    } else {
+      return cacheMap.get(adapterFactory.getClass()) == adapterFactory;
+    }
+  }
+
+  public void clearCache() {
+    fieldAdapterFactoryCache.remove();
   }
 
   @SuppressWarnings({ "unchecked", "rawtypes" }) // Casts guarded by conditionals.
   TypeAdapter<?> getTypeAdapter(ConstructorConstructor constructorConstructor, Gson gson,
-      TypeToken<?> type, JsonAdapter annotation) {
-    Object instance = constructorConstructor.get(TypeToken.get(annotation.value())).construct();
+      TypeToken<?> type, JsonAdapter annotation, boolean forField) {
+    Class<?> adapterClass = annotation.value();
+    TypeToken<?> adapterTypeToken = TypeToken.get(adapterClass);
+    Object instance = null;
+    if (forField && TypeAdapterFactory.class.isAssignableFrom(adapterClass)) {
+      Map<Class<?>, Object> cacheMap = fieldAdapterFactoryCache.get();
+      if (cacheMap == null) {
+        cacheMap = new HashMap<Class<?>, Object>();
+        fieldAdapterFactoryCache.set(cacheMap);
+      } else {
+        instance = cacheMap.get(adapterClass);
+      }
+
+      if (instance == null) {
+        instance = constructorConstructor.get(adapterTypeToken).construct();
+        cacheMap.put(adapterClass, instance);
+      }
+    } else {
+      instance = constructorConstructor.get(adapterTypeToken).construct();
+    }
 
     TypeAdapter<?> typeAdapter;
     if (instance instanceof TypeAdapter) {
@@ -66,7 +138,9 @@ TypeAdapter<?> getTypeAdapter(ConstructorConstructor constructorConstructor, Gso
       JsonDeserializer<?> deserializer = instance instanceof JsonDeserializer
           ? (JsonDeserializer) instance
           : null;
-      typeAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, null);
+
+      TypeAdapterFactory dummyFactory = forField ? DUMMY_FACTORY_FIELD : DUMMY_FACTORY_CLASS;
+      typeAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, dummyFactory);
     } else {
       throw new IllegalArgumentException("Invalid attempt to bind an instance of "
           + instance.getClass().getName() + " as a @JsonAdapter for " + type.toString()

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

@@ -61,11 +61,11 @@ public ReflectiveTypeAdapterFactory(ConstructorConstructor constructorConstructo
     this.jsonAdapterFactory = jsonAdapterFactory;
   }
 
-  public boolean excludeField(Field f, boolean serialize) {
-    return excludeField(f, serialize, excluder);
+  public boolean includeField(Field f, boolean serialize) {
+    return includeField(f, serialize, excluder);
   }
 
-  static boolean excludeField(Field f, boolean serialize, Excluder excluder) {
+  static boolean includeField(Field f, boolean serialize, Excluder excluder) {
     return !excluder.excludeClass(f.getType(), serialize) && !excluder.excludeField(f, serialize);
   }
 
@@ -111,7 +111,7 @@ private ReflectiveTypeAdapterFactory.BoundField createBoundField(
     TypeAdapter<?> mapped = null;
     if (annotation != null) {
       mapped = jsonAdapterFactory.getTypeAdapter(
-          constructorConstructor, context, fieldType, annotation);
+          constructorConstructor, context, fieldType, annotation, true);
     }
     final boolean jsonAdapterPresent = mapped != null;
     if (mapped == null) mapped = context.getAdapter(fieldType);
@@ -151,8 +151,8 @@ private Map<String, BoundField> getBoundFields(Gson context, TypeToken<?> type,
     while (raw != Object.class) {
       Field[] fields = raw.getDeclaredFields();
       for (Field field : fields) {
-        boolean serialize = excludeField(field, true);
-        boolean deserialize = excludeField(field, false);
+        boolean serialize = includeField(field, true);
+        boolean deserialize = includeField(field, false);
         if (!serialize && !deserialize) {
           continue;
         }

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

@@ -260,4 +260,79 @@ public void testIncorrectJsonAdapterType() {
   private static final class D {
     @SuppressWarnings("unused") final String value = "a";
   }
+
+  /**
+   * Verifies that {@link TypeAdapterFactory} specified by JsonAdapter can
+   * call Gson.getDelegateAdapter without any issues, despite the factory
+   * not being directly registered on Gson.
+   */
+  public void testDelegatingAdapterFactory() {
+    E deserialized = new Gson().fromJson("{\"f\":\"test\"}", E.class);
+    assertEquals("test", deserialized.f);
+  }
+  @JsonAdapter(E.DelegatingAdapterFactory.class)
+  private static class E {
+    String f;
+
+    static class DelegatingAdapterFactory implements TypeAdapterFactory {
+      @Override
+      public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
+        return gson.getDelegateAdapter(this, type);
+      }
+    }
+  }
+
+  /**
+   * Tests usage of {@link JsonSerializer} as {@link JsonAdapter} value
+   */
+  public void testJsonSerializer() {
+    Gson gson = new Gson();
+    // Verify that delegate deserializer (reflection deserializer) is used
+    JsonSerializerTest deserialized = gson.fromJson("{\"f\":\"test\"}", JsonSerializerTest.class);
+    assertEquals("test", deserialized.f);
+
+    String json = gson.toJson(new JsonSerializerTest());
+    // Uses custom serializer which always returns `true`
+    assertEquals("true", json);
+  }
+  @JsonAdapter(JsonSerializerTest.Serializer.class)
+  private static class JsonSerializerTest {
+    String f = "";
+
+    static class Serializer implements JsonSerializer<JsonSerializerTest> {
+      @Override
+      public JsonElement serialize(JsonSerializerTest src, Type typeOfSrc, JsonSerializationContext context) {
+        return new JsonPrimitive(true);
+      }
+    }
+  }
+
+  /**
+   * Tests usage of {@link JsonDeserializer} as {@link JsonAdapter} value
+   */
+  public void testJsonDeserializer() {
+    Gson gson = new Gson();
+    JsonDeserializerTest deserialized = gson.fromJson("{\"f\":\"test\"}", JsonDeserializerTest.class);
+    // Uses custom deserializer which always uses "123" as field value
+    assertEquals("123", deserialized.f);
+
+    // Verify that delegate serializer (reflection serializer) is used
+    String json = gson.toJson(new JsonDeserializerTest("abc"));
+    assertEquals("{\"f\":\"abc\"}", json);
+  }
+  @JsonAdapter(JsonDeserializerTest.Deserializer.class)
+  private static class JsonDeserializerTest {
+    String f;
+
+    JsonDeserializerTest(String f) {
+      this.f = f;
+    }
+
+    static class Deserializer implements JsonDeserializer<JsonDeserializerTest> {
+      @Override
+      public JsonDeserializerTest deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) {
+        return new JsonDeserializerTest("123");
+      }
+    }
+  }
 }