Improve JsonAdapter documentation and tests (#2442)

* Document how `JsonAdapter` creates adapter instances & add tests

* Extend `JsonAdapter.nullSafe()` documentation

* Improve test for JsonAdapter factory returning null

Existing test `JsonAdapterNullSafeTest` had misleading comments; while it
did in the end detect if null had not been handled correctly, that only
worked because the field `JsonAdapterFactory.recursiveCall` is static and
one test method therefore affected the state of the other test method.
If the test methods were run separately in different test runs, they would
not have detected if null was handled correctly, because the factory would
not have returned null.

* Extend JsonAdapter nullSafe test

* Extend test

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

@@ -73,6 +73,8 @@
  *
  * @param <T> the type of object that will be created by this implementation.
  *
+ * @see GsonBuilder#registerTypeAdapter(Type, Object)
+ *
  * @author Inderjeet Singh
  * @author Joel Leitch
  */

gson/src/main/java/com/google/gson/annotations/JsonAdapter.java

@@ -17,6 +17,8 @@
 package com.google.gson.annotations;
 
 import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
+import com.google.gson.InstanceCreator;
 import com.google.gson.JsonDeserializer;
 import com.google.gson.JsonSerializer;
 import com.google.gson.TypeAdapter;
@@ -35,20 +37,22 @@
  * @JsonAdapter(UserJsonAdapter.class)
  * public class User {
  *   public final String firstName, lastName;
+ *
  *   private User(String firstName, String lastName) {
  *     this.firstName = firstName;
  *     this.lastName = lastName;
  *   }
  * }
+ *
  * public class UserJsonAdapter extends TypeAdapter<User> {
  *   @Override public void write(JsonWriter out, User user) throws IOException {
  *     // implement write: combine firstName and lastName into name
  *     out.beginObject();
  *     out.name("name");
  *     out.value(user.firstName + " " + user.lastName);
  *     out.endObject();
- *     // implement the write method
  *   }
+ *
  *   @Override public User read(JsonReader in) throws IOException {
  *     // implement read: split name into firstName and lastName
  *     in.beginObject();
@@ -60,30 +64,46 @@
  * }
  * </pre>
  *
- * Since User class specified UserJsonAdapter.class in &#64;JsonAdapter annotation, it
- * will automatically be invoked to serialize/deserialize User instances.
+ * Since {@code User} class specified {@code UserJsonAdapter.class} in {@code @JsonAdapter}
+ * annotation, it will automatically be invoked to serialize/deserialize {@code User} instances.
  *
- * <p> Here is an example of how to apply this annotation to a field.
+ * <p>Here is an example of how to apply this annotation to a field.
  * <pre>
  * private static final class Gadget {
- *   &#64;JsonAdapter(UserJsonAdapter2.class)
+ *   &#64;JsonAdapter(UserJsonAdapter.class)
  *   final User user;
+ *
  *   Gadget(User user) {
  *     this.user = user;
  *   }
  * }
  * </pre>
  *
  * It's possible to specify different type adapters on a field, that
- * field's type, and in the {@link com.google.gson.GsonBuilder}. Field
- * annotations take precedence over {@code GsonBuilder}-registered type
+ * field's type, and in the {@link GsonBuilder}. Field annotations
+ * take precedence over {@code GsonBuilder}-registered type
  * adapters, which in turn take precedence over annotated types.
  *
  * <p>The class referenced by this annotation must be either a {@link
  * TypeAdapter} or a {@link TypeAdapterFactory}, or must implement one
  * or both of {@link JsonDeserializer} or {@link JsonSerializer}.
  * Using {@link TypeAdapterFactory} makes it possible to delegate
- * to the enclosing {@link Gson} instance.
+ * to the enclosing {@link Gson} instance. By default the specified
+ * adapter will not be called for {@code null} values; set {@link #nullSafe()}
+ * to {@code false} to let the adapter handle {@code null} values itself.
+ *
+ * <p>The type adapter is created in the same way Gson creates instances of
+ * custom classes during deserialization, that means:
+ * <ol>
+ *   <li>If a custom {@link InstanceCreator} has been registered for the
+ *       adapter class, it will be used to create the instance
+ *   <li>Otherwise, if the adapter class has a no-args constructor
+ *       (regardless of which visibility), it will be invoked to create
+ *       the instance
+ *   <li>Otherwise, JDK {@code Unsafe} will be used to create the instance;
+ *       see {@link GsonBuilder#disableJdkUnsafe()} for the unexpected
+ *       side-effects this might have
+ * </ol>
  *
  * <p>{@code Gson} instances might cache the adapter they create for
  * a {@code @JsonAdapter} annotation. It is not guaranteed that a new
@@ -104,7 +124,13 @@
   /** Either a {@link TypeAdapter} or {@link TypeAdapterFactory}, or one or both of {@link JsonDeserializer} or {@link JsonSerializer}. */
   Class<?> value();
 
-  /** false, to be able to handle {@code null} values within the adapter, default value is true. */
+  /**
+   * Whether the adapter referenced by {@link #value()} should be made {@linkplain TypeAdapter#nullSafe() null-safe}.
+   *
+   * <p>If {@code true} (the default), it will be made null-safe and Gson will handle {@code null} Java objects
+   * on serialization and JSON {@code null} on deserialization without calling the adapter. If {@code false},
+   * the adapter will have to handle the {@code null} values.
+   */
   boolean nullSafe() default true;
 
 }

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

@@ -134,6 +134,7 @@ TypeAdapter<?> getTypeAdapter(ConstructorConstructor constructorConstructor, Gso
       TypeAdapter<?> tempAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, skipPast, nullSafe);
       typeAdapter = tempAdapter;
 
+      // TreeTypeAdapter handles nullSafe; don't additionally call `nullSafe()`
       nullSafe = false;
     } else {
       throw new IllegalArgumentException("Invalid attempt to bind an instance of "

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

@@ -37,6 +37,7 @@
 import com.google.gson.stream.JsonReader;
 import com.google.gson.stream.JsonWriter;
 import java.io.IOException;
+import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.util.List;
 import java.util.Locale;
@@ -147,10 +148,84 @@ public void testSuperclassTypeAdapterNotInvoked() {
   }
 
   @Test
-  public void testNullSafeObjectFromJson() {
+  public void testNullSafeObject() {
     Gson gson = new Gson();
     NullableClass fromJson = gson.fromJson("null", NullableClass.class);
     assertThat(fromJson).isNull();
+
+    fromJson = gson.fromJson("\"ignored\"", NullableClass.class);
+    assertThat(fromJson).isNotNull();
+
+    String json = gson.toJson(null, NullableClass.class);
+    assertThat(json).isEqualTo("null");
+
+    json = gson.toJson(new NullableClass());
+    assertThat(json).isEqualTo("\"nullable\"");
+  }
+
+  /**
+   * Tests behavior when a {@link TypeAdapterFactory} registered with {@code @JsonAdapter} returns
+   * {@code null}, indicating that it cannot handle the type and Gson should try a different factory
+   * instead.
+   */
+  @Test
+  public void testFactoryReturningNull() {
+    Gson gson = new Gson();
+
+    assertThat(gson.fromJson("null", WithNullReturningFactory.class)).isNull();
+    assertThat(gson.toJson(null, WithNullReturningFactory.class)).isEqualTo("null");
+
+    TypeToken<WithNullReturningFactory<String>> stringTypeArg = new TypeToken<WithNullReturningFactory<String>>() {};
+    WithNullReturningFactory<?> deserialized = gson.fromJson("\"a\"", stringTypeArg);
+    assertThat(deserialized.t).isEqualTo("custom-read:a");
+    assertThat(gson.fromJson("null", stringTypeArg)).isNull();
+    assertThat(gson.toJson(new WithNullReturningFactory<>("b"), stringTypeArg.getType())).isEqualTo("\"custom-write:b\"");
+    assertThat(gson.toJson(null, stringTypeArg.getType())).isEqualTo("null");
+
+    // Factory should return `null` for this type and Gson should fall back to reflection-based adapter
+    TypeToken<WithNullReturningFactory<Integer>> numberTypeArg = new TypeToken<WithNullReturningFactory<Integer>>() {};
+    deserialized = gson.fromJson("{\"t\":1}", numberTypeArg);
+    assertThat(deserialized.t).isEqualTo(1);
+    assertThat(gson.toJson(new WithNullReturningFactory<>(2), numberTypeArg.getType())).isEqualTo("{\"t\":2}");
+  }
+  // Also set `nullSafe = true` to verify that this does not cause a NullPointerException if the
+  // factory would accidentally call `nullSafe()` on null adapter
+  @JsonAdapter(value = WithNullReturningFactory.NullReturningFactory.class, nullSafe = true)
+  private static class WithNullReturningFactory<T> {
+    T t;
+
+    public WithNullReturningFactory(T t) {
+      this.t = t;
+    }
+
+    static class NullReturningFactory implements TypeAdapterFactory {
+      @Override
+      public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
+        // Don't handle raw (non-parameterized) type
+        if (type.getType() instanceof Class) {
+          return null;
+        }
+        ParameterizedType parameterizedType = (ParameterizedType) type.getType();
+        // Makes this test a bit more realistic by only conditionally returning null (instead of always)
+        if (parameterizedType.getActualTypeArguments()[0] != String.class) {
+          return null;
+        }
+
+        @SuppressWarnings("unchecked")
+        TypeAdapter<T> adapter = (TypeAdapter<T>) new TypeAdapter<WithNullReturningFactory<String>>() {
+          @Override
+          public void write(JsonWriter out, WithNullReturningFactory<String> value) throws IOException {
+            out.value("custom-write:" + value.t);
+          }
+
+          @Override
+          public WithNullReturningFactory<String> read(JsonReader in) throws IOException {
+            return new WithNullReturningFactory<>("custom-read:" + in.nextString());
+          }
+        };
+        return adapter;
+      }
+    }
   }
 
   @JsonAdapter(A.JsonAdapter.class)
@@ -223,7 +298,6 @@ private static class UserJsonAdapter extends TypeAdapter<User> {
       out.name("name");
       out.value(user.firstName + " " + user.lastName);
       out.endObject();
-      // implement the write method
     }
     @Override public User read(JsonReader in) throws IOException {
       // implement read: split name into firstName and lastName
@@ -235,6 +309,7 @@ private static class UserJsonAdapter extends TypeAdapter<User> {
     }
   }
 
+  // Implicit `nullSafe=true`
   @JsonAdapter(value = NullableClassJsonAdapter.class)
   private static class NullableClass {
   }
@@ -606,4 +681,65 @@ public WithJsonDeserializer deserialize(JsonElement json, Type typeOfT, JsonDese
       }
     }
   }
+
+  /**
+   * Tests creation of the adapter referenced by {@code @JsonAdapter} using an {@link InstanceCreator}.
+   */
+  @Test
+  public void testAdapterCreatedByInstanceCreator() {
+    CreatedByInstanceCreator.Serializer serializer = new CreatedByInstanceCreator.Serializer("custom");
+    Gson gson = new GsonBuilder()
+        .registerTypeAdapter(CreatedByInstanceCreator.Serializer.class, (InstanceCreator<?>) t -> serializer)
+        .create();
+
+    String json = gson.toJson(new CreatedByInstanceCreator());
+    assertThat(json).isEqualTo("\"custom\"");
+  }
+  @JsonAdapter(CreatedByInstanceCreator.Serializer.class)
+  private static class CreatedByInstanceCreator {
+    static class Serializer implements JsonSerializer<CreatedByInstanceCreator> {
+      private final String value;
+
+      @SuppressWarnings("unused")
+      public Serializer() {
+        throw new AssertionError("should not be called");
+      }
+
+      public Serializer(String value) {
+        this.value = value;
+      }
+
+      @Override
+      public JsonElement serialize(CreatedByInstanceCreator src, Type typeOfSrc, JsonSerializationContext context) {
+        return new JsonPrimitive(value);
+      }
+    }
+  }
+
+  /**
+   * Tests creation of the adapter referenced by {@code @JsonAdapter} using JDK Unsafe.
+   */
+  @Test
+  public void testAdapterCreatedByJdkUnsafe() {
+    String json = new Gson().toJson(new CreatedByJdkUnsafe());
+    assertThat(json).isEqualTo("false");
+  }
+  @JsonAdapter(CreatedByJdkUnsafe.Serializer.class)
+  private static class CreatedByJdkUnsafe {
+    static class Serializer implements JsonSerializer<CreatedByJdkUnsafe> {
+      // JDK Unsafe leaves this at default value `false`
+      private boolean wasInitialized = true;
+
+      // Explicit constructor with args to remove implicit no-args constructor
+      @SuppressWarnings("unused")
+      public Serializer(int i) {
+        throw new AssertionError("should not be called");
+      }
+
+      @Override
+      public JsonElement serialize(CreatedByJdkUnsafe src, Type typeOfSrc, JsonSerializationContext context) {
+        return new JsonPrimitive(wasInitialized);
+      }
+    }
+  }
 }

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

@@ -20,14 +20,19 @@
 
 import com.google.errorprone.annotations.Keep;
 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.JsonParseException;
 import com.google.gson.JsonPrimitive;
 import com.google.gson.JsonSerializationContext;
 import com.google.gson.JsonSerializer;
+import com.google.gson.TypeAdapter;
 import com.google.gson.annotations.JsonAdapter;
+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;
 
@@ -43,7 +48,7 @@ public void testJsonSerializerDeserializerBasedJsonAdapterOnFields() {
     String json = gson.toJson(new Computer(new User("Inderjeet Singh"), null, new User("Jesse Wilson")));
     assertThat(json).isEqualTo("{\"user1\":\"UserSerializer\",\"user3\":\"UserSerializerDeserializer\"}");
     Computer computer = gson.fromJson("{'user2':'Jesse Wilson','user3':'Jake Wharton'}", Computer.class);
-    assertThat(computer.user2.name).isEqualTo("UserSerializer");
+    assertThat(computer.user2.name).isEqualTo("UserDeserializer");
     assertThat(computer.user3.name).isEqualTo("UserSerializerDeserializer");
   }
 
@@ -82,7 +87,7 @@ private static final class UserDeserializer implements JsonDeserializer<User> {
     @Override
     public User deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context)
         throws JsonParseException {
-      return new User("UserSerializer");
+      return new User("UserDeserializer");
     }
   }
 
@@ -178,20 +183,48 @@ private static final class BaseIntegerAdapter implements JsonSerializer<Base<Int
 
   @Test
   public void testJsonAdapterNullSafe() {
-    Gson gson = new Gson();
-    String json = gson.toJson(new Computer3(null, null));
-    assertThat(json).isEqualTo("{\"user1\":\"UserSerializerDeserializer\"}");
-    Computer3 computer3 = gson.fromJson("{\"user1\":null, \"user2\":null}", Computer3.class);
-    assertThat(computer3.user1.name).isEqualTo("UserSerializerDeserializer");
-    assertThat(computer3.user2).isNull();
-  }
-
-  private static final class Computer3 {
-    @JsonAdapter(value = UserSerializerDeserializer.class, nullSafe = false) final User user1;
-    @JsonAdapter(value = UserSerializerDeserializer.class) final User user2;
-    Computer3(User user1, User user2) {
-      this.user1 = user1;
-      this.user2 = user2;
+    Gson gson = new GsonBuilder()
+        .registerTypeAdapter(User.class, new TypeAdapter<User>() {
+          @Override
+          public User read(JsonReader in) throws IOException {
+            in.nextNull();
+            return new User("fallback-read");
+          }
+          @Override
+          public void write(JsonWriter out, User value) throws IOException {
+            assertThat(value).isNull();
+            out.value("fallback-write");
+          }
+        })
+        .serializeNulls()
+        .create();
+
+    String json = gson.toJson(new WithNullSafe(null, null, null, null));
+    // Only nullSafe=true serializer writes null; for @JsonAdapter with deserializer nullSafe is ignored when serializing
+    assertThat(json).isEqualTo("{\"userS\":\"UserSerializer\",\"userSN\":null,\"userD\":\"fallback-write\",\"userDN\":\"fallback-write\"}");
+
+    WithNullSafe deserialized = gson.fromJson("{\"userS\":null,\"userSN\":null,\"userD\":null,\"userDN\":null}", WithNullSafe.class);
+    // For @JsonAdapter with serializer nullSafe is ignored when deserializing
+    assertThat(deserialized.userS.name).isEqualTo("fallback-read");
+    assertThat(deserialized.userSN.name).isEqualTo("fallback-read");
+    assertThat(deserialized.userD.name).isEqualTo("UserDeserializer");
+    assertThat(deserialized.userDN).isNull();
+  }
+
+  private static final class WithNullSafe {
+    // "userS..." uses JsonSerializer
+    @JsonAdapter(value = UserSerializer.class, nullSafe = false) final User userS;
+    @JsonAdapter(value = UserSerializer.class, nullSafe = true) final User userSN;
+
+    // "userD..." uses JsonDeserializer
+    @JsonAdapter(value = UserDeserializer.class, nullSafe = false) final User userD;
+    @JsonAdapter(value = UserDeserializer.class, nullSafe = true) final User userDN;
+
+    WithNullSafe(User userS, User userSN, User userD, User userDN) {
+      this.userS = userS;
+      this.userSN = userSN;
+      this.userD = userD;
+      this.userDN = userDN;
     }
   }
 }