Fix Gson.getDelegateAdapter not working properly for JsonAdapter

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

@@ -16,6 +16,7 @@
 
 package com.google.gson;
 
+import com.google.gson.annotations.JsonAdapter;
 import com.google.gson.internal.ConstructorConstructor;
 import com.google.gson.internal.Excluder;
 import com.google.gson.internal.GsonBuildConfig;
@@ -593,54 +594,71 @@ public <T> TypeAdapter<T> getAdapter(TypeToken<T> type) {
    * adapter that does a little bit of work but then delegates further processing to the Gson
    * default type adapter. Here is an example:
    * <p>Let's say we want to write a type adapter that counts the number of objects being read
-   *  from or written to JSON. We can achieve this by writing a type adapter factory that uses
-   *  the <code>getDelegateAdapter</code> method:
-   *  <pre> {@code
-   *  class StatsTypeAdapterFactory implements TypeAdapterFactory {
-   *    public int numReads = 0;
-   *    public int numWrites = 0;
-   *    public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
-   *      final TypeAdapter<T> delegate = gson.getDelegateAdapter(this, type);
-   *      return new TypeAdapter<T>() {
-   *        public void write(JsonWriter out, T value) throws IOException {
-   *          ++numWrites;
-   *          delegate.write(out, value);
-   *        }
-   *        public T read(JsonReader in) throws IOException {
-   *          ++numReads;
-   *          return delegate.read(in);
-   *        }
-   *      };
-   *    }
-   *  }
-   *  } </pre>
-   *  This factory can now be used like this:
-   *  <pre> {@code
-   *  StatsTypeAdapterFactory stats = new StatsTypeAdapterFactory();
-   *  Gson gson = new GsonBuilder().registerTypeAdapterFactory(stats).create();
-   *  // Call gson.toJson() and fromJson methods on objects
-   *  System.out.println("Num JSON reads" + stats.numReads);
-   *  System.out.println("Num JSON writes" + stats.numWrites);
-   *  }</pre>
-   *  Note that this call will skip all factories registered before {@code skipPast}. In case of
-   *  multiple TypeAdapterFactories registered it is up to the caller of this function to insure
-   *  that the order of registration does not prevent this method from reaching a factory they
-   *  would expect to reply from this call.
-   *  Note that since you can not override type adapter factories for String and Java primitive
-   *  types, our stats factory will not count the number of String or primitives that will be
-   *  read or written.
+   * from or written to JSON. We can achieve this by writing a type adapter factory that uses
+   * the <code>getDelegateAdapter</code> method:
+   * <pre>{@code
+   * class StatsTypeAdapterFactory implements TypeAdapterFactory {
+   *   public int numReads = 0;
+   *   public int numWrites = 0;
+   *   public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
+   *     final TypeAdapter<T> delegate = gson.getDelegateAdapter(this, type);
+   *     return new TypeAdapter<T>() {
+   *       public void write(JsonWriter out, T value) throws IOException {
+   *         ++numWrites;
+   *         delegate.write(out, value);
+   *       }
+   *       public T read(JsonReader in) throws IOException {
+   *         ++numReads;
+   *         return delegate.read(in);
+   *       }
+   *     };
+   *   }
+   * }
+   * }</pre>
+   * This factory can now be used like this:
+   * <pre>{@code
+   * StatsTypeAdapterFactory stats = new StatsTypeAdapterFactory();
+   * Gson gson = new GsonBuilder().registerTypeAdapterFactory(stats).create();
+   * // Call gson.toJson() and fromJson methods on objects
+   * System.out.println("Num JSON reads" + stats.numReads);
+   * System.out.println("Num JSON writes" + stats.numWrites);
+   * }</pre>
+   * Note that this call will skip all factories registered before {@code skipPast}. In case of
+   * multiple TypeAdapterFactories registered it is up to the caller of this function to insure
+   * that the order of registration does not prevent this method from reaching a factory they
+   * would expect to reply from this call.
+   * Note that since you can not override type adapter factories for String and Java primitive
+   * types, our stats factory will not count the number of String or primitives that will be
+   * read or written.
+   *
+   * <p>If {@code skipPast} is {@code null} or a factory which has neither been registered
+   * on the {@link GsonBuilder} nor specified with the {@link JsonAdapter @JsonAdapter} annotation
+   * on a class, then this method behaves as if {@link #getAdapter(TypeToken)} had been called.
+   * This also means that for fields with {@code @JsonAdapter} annotation this method behaves
+   * normally like {@code getAdapter} (except for corner cases where a custom {@link InstanceCreator}
+   * is used to create an instance of the factory).
+   *
    * @param skipPast The type adapter factory that needs to be skipped while searching for
    *   a matching type adapter. In most cases, you should just pass <i>this</i> (the type adapter
-   *   factory from where {@code getDelegateAdapter} method is being invoked).
+   *   factory from where {@code getDelegateAdapter} method is being invoked). May be {@code null}.
    * @param type Type for which the delegate adapter is being searched for.
    *
    * @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;
+    Objects.requireNonNull(type, "type must not be null");
+
+    if (skipPast != null) {
+      if (jsonAdapterFactory.isClassJsonAdapterFactory(type.getRawType(), skipPast)) {
+        skipPast = jsonAdapterFactory;
+      } else if (!factories.contains(skipPast)) {
+        // Probably a factory from @JsonAdapter on a field
+        skipPast = null;
+      }
+    }
+
+    if (skipPast == null) {
+      return getAdapter(type);
     }
 
     boolean skipPastFound = false;

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

@@ -63,7 +63,7 @@
  * </pre>
  *
  * <p>Note that it does not matter what the fields of the created instance contain since Gson will
- * overwrite them with the deserialized values specified in Json. You should also ensure that a
+ * overwrite them with the deserialized values specified in JSON. You should also ensure that a
  * <i>new</i> object is returned, not a common object since its fields will be overwritten.
  * The developer will need to register {@code IdInstanceCreator} with Gson as follows:</p>
  *
@@ -81,7 +81,7 @@ public interface InstanceCreator<T> {
   /**
    * Gson invokes this call-back method during deserialization to create an instance of the
    * specified type. The fields of the returned instance are overwritten with the data present
-   * in the Json. Since the prior contents of the object are destroyed and overwritten, do not
+   * in the JSON. Since the prior contents of the object are destroyed and overwritten, do not
    * return an instance that is useful elsewhere. In particular, do not return a common instance,
    * always use {@code new} to create a new instance.
    *

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

@@ -88,6 +88,19 @@ static String checkInstantiable(Class<?> c) {
     return null;
   }
 
+  private static <T> T useInstanceCreator(InstanceCreator<T> instanceCreator, Type type, Class<?> rawType) {
+    T instance = instanceCreator.createInstance(type);
+    if (instance == null) {
+      throw new RuntimeException("InstanceCreator " + instanceCreator + " returned null for type " + type);
+    }
+
+    if (!rawType.isInstance(instance)) {
+      throw new ClassCastException("InstanceCreator " + instanceCreator + " created instance of wrong type;"
+          + " expected " + rawType.getName() + " but got instance of unrelated type " + instance.getClass().getName());
+    }
+    return instance;
+  }
+
   public <T> ObjectConstructor<T> get(TypeToken<T> typeToken) {
     final Type type = typeToken.getType();
     final Class<? super T> rawType = typeToken.getRawType();
@@ -99,7 +112,7 @@ public <T> ObjectConstructor<T> get(TypeToken<T> typeToken) {
     if (typeCreator != null) {
       return new ObjectConstructor<T>() {
         @Override public T construct() {
-          return typeCreator.createInstance(type);
+          return useInstanceCreator(typeCreator, type, rawType);
         }
       };
     }
@@ -111,7 +124,7 @@ public <T> ObjectConstructor<T> get(TypeToken<T> typeToken) {
     if (rawTypeCreator != null) {
       return new ObjectConstructor<T>() {
         @Override public T construct() {
-          return rawTypeCreator.createInstance(type);
+          return useInstanceCreator(rawTypeCreator, type, rawType);
         }
       };
     }

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

@@ -24,6 +24,9 @@
 import com.google.gson.annotations.JsonAdapter;
 import com.google.gson.internal.ConstructorConstructor;
 import com.google.gson.reflect.TypeToken;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * Given a type T, looks for the annotation {@link JsonAdapter} and uses an instance of the
@@ -32,35 +35,76 @@
  * @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 {@code @JsonAdapter}
+   * on a class.
+   */
+  private static final TypeAdapterFactory TREE_TYPE_DUMMY_FACTORY = new DummyTypeAdapterFactory();
+
   private final ConstructorConstructor constructorConstructor;
+  /**
+   * For a class, if it is annotated with {@code @JsonAdapter} and refers to a {@code TypeAdapterFactory}
+   * stores the factory instance, in case it has been requested already.
+   */
+  // Note: In case these strong reference to TypeAdapterFactory instances are considered
+  // a memory leak in the future, could consider switching to WeakReference<TypeAdapterFactory>
+  private final ConcurrentMap<Class<?>, TypeAdapterFactory> adapterFactoryMap;
 
   public JsonAdapterAnnotationTypeAdapterFactory(ConstructorConstructor constructorConstructor) {
     this.constructorConstructor = constructorConstructor;
+    this.adapterFactoryMap = new ConcurrentHashMap<>();
+  }
+
+  // Separate helper method to make sure callers retrieve annotation in a consistent way
+  private JsonAdapter getAnnotation(Class<?> rawType) {
+    return rawType.getAnnotation(JsonAdapter.class);
   }
 
   @SuppressWarnings("unchecked") // this is not safe; requires that user has specified correct adapter class for @JsonAdapter
   @Override
   public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> targetType) {
     Class<? super T> rawType = targetType.getRawType();
-    JsonAdapter annotation = rawType.getAnnotation(JsonAdapter.class);
+    JsonAdapter annotation = getAnnotation(rawType);
     if (annotation == null) {
       return null;
     }
-    return (TypeAdapter<T>) getTypeAdapter(constructorConstructor, gson, targetType, annotation);
+    return (TypeAdapter<T>) getTypeAdapter(constructorConstructor, gson, targetType, annotation, true);
   }
 
-  TypeAdapter<?> getTypeAdapter(ConstructorConstructor constructorConstructor, Gson gson,
-      TypeToken<?> type, JsonAdapter annotation) {
+  // Separate helper method to make sure callers create adapter in a consistent way
+  private static Object createAdapter(ConstructorConstructor constructorConstructor, Class<?> adapterClass) {
     // TODO: The exception messages created by ConstructorConstructor are currently written in the context of
     // deserialization and for example suggest usage of TypeAdapter, which would not work for @JsonAdapter usage
-    Object instance = constructorConstructor.get(TypeToken.get(annotation.value())).construct();
+    return constructorConstructor.get(TypeToken.get(adapterClass)).construct();
+  }
+
+  private TypeAdapterFactory putFactoryAndGetCurrent(Class<?> rawType, TypeAdapterFactory factory) {
+    // Uses putIfAbsent in case multiple threads concurrently create factory
+    TypeAdapterFactory existingFactory = adapterFactoryMap.putIfAbsent(rawType, factory);
+    return existingFactory != null ? existingFactory : factory;
+  }
+
+  TypeAdapter<?> getTypeAdapter(ConstructorConstructor constructorConstructor, Gson gson,
+      TypeToken<?> type, JsonAdapter annotation, boolean isClassAnnotation) {
+    Object instance = createAdapter(constructorConstructor, annotation.value());
 
     TypeAdapter<?> typeAdapter;
     boolean nullSafe = annotation.nullSafe();
     if (instance instanceof TypeAdapter) {
       typeAdapter = (TypeAdapter<?>) instance;
     } else if (instance instanceof TypeAdapterFactory) {
-      typeAdapter = ((TypeAdapterFactory) instance).create(gson, type);
+      TypeAdapterFactory factory = ((TypeAdapterFactory) instance);
+
+      if (isClassAnnotation) {
+        factory = putFactoryAndGetCurrent(type.getRawType(), factory);
+      }
+
+      typeAdapter = factory.create(gson, type);
     } else if (instance instanceof JsonSerializer || instance instanceof JsonDeserializer) {
       JsonSerializer<?> serializer = instance instanceof JsonSerializer
           ? (JsonSerializer<?>) instance
@@ -69,8 +113,15 @@ TypeAdapter<?> getTypeAdapter(ConstructorConstructor constructorConstructor, Gso
           ? (JsonDeserializer<?>) instance
           : null;
 
+      TypeAdapterFactory skipPast = null;
+      if (isClassAnnotation) {
+        // Use dummy `skipPast` value; otherwise TreeTypeAdapter's call to `Gson.getDelegateAdapter` would
+        // cause infinite recursion because it would keep returning adapter specified by @JsonAdapter
+        skipPast = TREE_TYPE_DUMMY_FACTORY;
+        adapterFactoryMap.put(type.getRawType(), skipPast);
+      }
       @SuppressWarnings({ "unchecked", "rawtypes" })
-      TypeAdapter<?> tempAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, null, nullSafe);
+      TypeAdapter<?> tempAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, skipPast, nullSafe);
       typeAdapter = tempAdapter;
 
       nullSafe = false;
@@ -87,4 +138,38 @@ TypeAdapter<?> getTypeAdapter(ConstructorConstructor constructorConstructor, Gso
 
     return typeAdapter;
   }
+
+  /**
+   * Returns whether {@code factory} is a type adapter factory created for {@code @JsonAdapter}
+   * placed on {@code rawType}.
+   */
+  public boolean isClassJsonAdapterFactory(Class<?> rawType, TypeAdapterFactory factory) {
+    Objects.requireNonNull(rawType);
+    Objects.requireNonNull(factory);
+
+    TypeAdapterFactory existingFactory = adapterFactoryMap.get(rawType);
+    if (existingFactory != null) {
+      // Checks for reference equality, like it is done by `Gson.getDelegateAdapter`
+      return existingFactory == factory;
+    }
+
+    // If no factory has been created for the type yet check manually for a @JsonAdapter annotation
+    // which specifies a TypeAdapterFactory
+    // Otherwise behavior would not be consistent, depending on whether or not adapter had been requested
+    // before call to `isClassJsonAdapterFactory` was made
+    JsonAdapter annotation = getAnnotation(rawType);
+    if (annotation == null) {
+      return false;
+    }
+
+    Class<?> adapterClass = annotation.value();
+    if (!TypeAdapterFactory.class.isAssignableFrom(adapterClass)) {
+      return false;
+    }
+
+    Object adapter = createAdapter(constructorConstructor, adapterClass);
+    TypeAdapterFactory newFactory = (TypeAdapterFactory) adapter;
+
+    return putFactoryAndGetCurrent(rawType, newFactory) == factory;
+  }
 }

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

@@ -156,7 +156,7 @@ private BoundField createBoundField(
     if (annotation != null) {
       // This is not safe; requires that user has specified correct adapter class for @JsonAdapter
       mapped = jsonAdapterFactory.getTypeAdapter(
-          constructorConstructor, context, fieldType, annotation);
+          constructorConstructor, context, fieldType, annotation, false);
     }
     final boolean jsonAdapterPresent = mapped != null;
     if (mapped == null) mapped = context.getAdapter(fieldType);

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

@@ -43,6 +43,10 @@ public final class TreeTypeAdapter<T> extends SerializationDelegatingTypeAdapter
   private final JsonDeserializer<T> deserializer;
   final Gson gson;
   private final TypeToken<T> typeToken;
+  /**
+   * Only intended as {@code skipPast} for {@link Gson#getDelegateAdapter(TypeAdapterFactory, TypeToken)},
+   * must not be used in any other way.
+   */
   private final TypeAdapterFactory skipPast;
   private final GsonContextImpl context = new GsonContextImpl();
   private final boolean nullSafe;