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 @@ import java.lang.reflect.Type;
  *
  * @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,11 +37,13 @@ import java.lang.annotation.Target;
  * @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
@@ -47,8 +51,8 @@ import java.lang.annotation.Target;
  *     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,14 +64,15 @@ import java.lang.annotation.Target;
  * }
  * </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;
  *   }
@@ -75,15 +80,30 @@ import java.lang.annotation.Target;
  * </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 @@ public @interface JsonAdapter {
   /** 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 @@ public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapte
       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.reflect.TypeToken;
 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 final class JsonAdapterAnnotationOnClassesTest {
   }
 
   @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 @@ public final class JsonAdapterAnnotationOnClassesTest {
       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 @@ public final class JsonAdapterAnnotationOnClassesTest {
     }
   }
 
+  // Implicit `nullSafe=true`
   @JsonAdapter(value = NullableClassJsonAdapter.class)
   private static class NullableClass {
   }
@@ -606,4 +681,65 @@ public final class JsonAdapterAnnotationOnClassesTest {
       }
     }
   }
+
+  /**
+   * 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,6 +20,7 @@ import static com.google.common.truth.Truth.assertThat;
 
 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;
@@ -27,7 +28,11 @@ 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 final class JsonAdapterSerializerDeserializerTest {
     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 @@ public final class JsonAdapterSerializerDeserializerTest {
     @Override
     public User deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context)
         throws JsonParseException {
-      return new User("UserSerializer");
+      return new User("UserDeserializer");
     }
   }
 
@@ -178,20 +183,48 @@ public final class JsonAdapterSerializerDeserializerTest {
 
   @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();
+    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 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;
+  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;
     }
   }
 }

gson/src/test/java/com/google/gson/regression/JsonAdapterNullSafeTest.java

@@ -1,64 +0,0 @@
-/*
- * Copyright (C) 2016 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.google.gson.regression;
-
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.gson.Gson;
-import com.google.gson.TypeAdapter;
-import com.google.gson.TypeAdapterFactory;
-import com.google.gson.annotations.JsonAdapter;
-import com.google.gson.reflect.TypeToken;
-import org.junit.Test;
-
-public class JsonAdapterNullSafeTest {
-  private final Gson gson = new Gson();
-
-  @Test
-  public void testNullSafeBugSerialize() {
-    Device device = new Device("ec57803e");
-    String unused = gson.toJson(device);
-  }
-
-  @Test
-  public void testNullSafeBugDeserialize() {
-    Device device = gson.fromJson("{'id':'ec57803e2'}", Device.class);
-    assertThat(device.id).isEqualTo("ec57803e2");
-  }
-
-  @JsonAdapter(Device.JsonAdapterFactory.class)
-  private static final class Device {
-    String id;
-    Device(String id) {
-      this.id = id;
-    }
-
-    static final class JsonAdapterFactory implements TypeAdapterFactory {
-      // The recursiveCall in {@link Device.JsonAdapterFactory} is the source of this bug
-      // because we use it to return a null type adapter on a recursive call.
-      private static final ThreadLocal<Boolean> recursiveCall = new ThreadLocal<>();
-
-      @Override public <T> TypeAdapter<T> create(final Gson gson, TypeToken<T> type) {
-        if (type.getRawType() != Device.class || recursiveCall.get() != null) {
-          recursiveCall.set(null); // clear for subsequent use
-          return null;
-        }
-        recursiveCall.set(Boolean.TRUE);
-        return gson.getDelegateAdapter(this, type);
-      }
-    }
-  }
-}