From 45511fdd1534aa4fcc3c560ac4c0b439d94fc2ad Mon Sep 17 00:00:00 2001 From: Inderjeet Singh Date: Wed, 18 May 2016 18:21:39 -0700 Subject: [PATCH 1/4] Added support for JsonSerializer/JsonDeserializer for JsonAdapter annotation. JsonAdapter is cached per the type of the JsonAdapter class. Added a test to ensure JsonAdapter works on fields of parameterized types Keep track of registered JsonAdapters and JsonAdapterFactorys in ThreadLocal. --- gson/src/main/java/com/google/gson/Gson.java | 24 +-- ...onAdapterAnnotationTypeAdapterFactory.java | 97 +++++++++-- .../bind/ReflectiveTypeAdapterFactory.java | 10 +- .../JsonAdapterAnnotationOnFieldsTest.java | 37 +++- ...JsonAdapterSerializerDeserializerTest.java | 164 ++++++++++++++++++ ...ntimeTypeAdapterFactoryFunctionalTest.java | 2 +- 6 files changed, 305 insertions(+), 29 deletions(-) create mode 100644 gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java diff --git a/gson/src/main/java/com/google/gson/Gson.java b/gson/src/main/java/com/google/gson/Gson.java index fa89794a..c34c896d 100644 --- a/gson/src/main/java/com/google/gson/Gson.java +++ b/gson/src/main/java/com/google/gson/Gson.java @@ -134,6 +134,7 @@ public final class Gson { private final boolean generateNonExecutableJson; private final boolean prettyPrinting; private final boolean lenient; + private JsonAdapterAnnotationTypeAdapterFactory jsonAdapterFactory; /** * Constructs a Gson object with default configuration. The default configuration has the @@ -245,10 +246,11 @@ public final class Gson { // type adapters for composite and user-defined types factories.add(new CollectionTypeAdapterFactory(constructorConstructor)); factories.add(new MapTypeAdapterFactory(constructorConstructor, complexMapKeySerialization)); - factories.add(new JsonAdapterAnnotationTypeAdapterFactory(constructorConstructor)); + this.jsonAdapterFactory = new JsonAdapterAnnotationTypeAdapterFactory(constructorConstructor); + factories.add(jsonAdapterFactory); factories.add(TypeAdapters.ENUM_FACTORY); factories.add(new ReflectiveTypeAdapterFactory( - constructorConstructor, fieldNamingStrategy, excluder)); + constructorConstructor, fieldNamingStrategy, excluder, jsonAdapterFactory)); this.factories = Collections.unmodifiableList(factories); } @@ -486,26 +488,26 @@ public final class Gson { * @since 2.2 */ public TypeAdapter getDelegateAdapter(TypeAdapterFactory skipPast, TypeToken type) { - boolean skipPastFound = false; - // Skip past if and only if the specified factory is present in the factories. - // This is useful because the factories created through JsonAdapter annotations are not - // registered in this list. - if (!factories.contains(skipPast)) skipPastFound = true; + // If the specified skipPast factory is not registered, ignore it. + boolean skipPastFound = skipPast == null + || (!factories.contains(skipPast) && jsonAdapterFactory.getDelegateAdapterFactory(type) == null); for (TypeAdapterFactory factory : factories) { if (!skipPastFound) { - if (factory == skipPast) { - skipPastFound = true; + skipPastFound = factory == skipPast; + if (!skipPastFound && factory instanceof JsonAdapterAnnotationTypeAdapterFactory) { + // Also check if there is a registered JsonAdapter for it + factory = ((JsonAdapterAnnotationTypeAdapterFactory)factory).getDelegateAdapterFactory(type); + skipPastFound = factory == skipPast; } continue; } - TypeAdapter candidate = factory.create(this, type); if (candidate != null) { return candidate; } } - throw new IllegalArgumentException("GSON cannot serialize " + type); + throw new IllegalArgumentException("GSON cannot serialize or deserialize " + type); } /** diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java index b52e1573..9de6b041 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java @@ -16,7 +16,12 @@ 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; import com.google.gson.TypeAdapter; import com.google.gson.TypeAdapterFactory; import com.google.gson.annotations.JsonAdapter; @@ -31,6 +36,21 @@ import com.google.gson.reflect.TypeToken; */ public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapterFactory { + @SuppressWarnings("rawtypes") + private final ThreadLocal> activeJsonAdapterClasses = new ThreadLocal>() { + @Override protected Map initialValue() { + // No need for a thread-safe map since we are using it in a single thread + return new HashMap(); + } + }; + @SuppressWarnings("rawtypes") + private final ThreadLocal> activeJsonAdapterFactories = new ThreadLocal>() { + @Override protected Map initialValue() { + // No need for a thread-safe map since we are using it in a single thread + return new HashMap(); + } + }; + private final ConstructorConstructor constructorConstructor; public JsonAdapterAnnotationTypeAdapterFactory(ConstructorConstructor constructorConstructor) { @@ -40,33 +60,86 @@ public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapte @SuppressWarnings("unchecked") @Override public TypeAdapter create(Gson gson, TypeToken targetType) { - JsonAdapter annotation = targetType.getRawType().getAnnotation(JsonAdapter.class); + Class rawType = targetType.getRawType(); + JsonAdapter annotation = rawType.getAnnotation(JsonAdapter.class); if (annotation == null) { return null; } return (TypeAdapter) getTypeAdapter(constructorConstructor, gson, targetType, annotation); } - @SuppressWarnings("unchecked") // Casts guarded by conditionals. - static TypeAdapter getTypeAdapter(ConstructorConstructor constructorConstructor, Gson gson, - TypeToken fieldType, JsonAdapter annotation) { + public TypeAdapter getDelegateAdapter(Gson gson, TypeAdapterFactory skipPast, TypeToken targetType) { + TypeAdapterFactory factory = getDelegateAdapterFactory(targetType); + if (factory == skipPast) factory = null; + return factory == null ? null: factory.create(gson, targetType); + } + + public TypeAdapterFactory getDelegateAdapterFactory(TypeToken targetType) { + Class annotatedClass = targetType.getRawType(); + JsonAdapter annotation = annotatedClass.getAnnotation(JsonAdapter.class); + if (annotation == null) { + return null; + } + return getTypeAdapterFactory(annotation, constructorConstructor); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) // Casts guarded by conditionals. + TypeAdapter getTypeAdapter(ConstructorConstructor constructorConstructor, Gson gson, + TypeToken type, JsonAdapter annotation) { Class value = annotation.value(); + boolean isTypeAdapter = TypeAdapter.class.isAssignableFrom(value); + boolean isJsonSerializer = JsonSerializer.class.isAssignableFrom(value); + boolean isJsonDeserializer = JsonDeserializer.class.isAssignableFrom(value); + TypeAdapter typeAdapter; - if (TypeAdapter.class.isAssignableFrom(value)) { - Class> typeAdapterClass = (Class>) value; - typeAdapter = constructorConstructor.get(TypeToken.get(typeAdapterClass)).construct(); + if (isTypeAdapter || isJsonSerializer || isJsonDeserializer) { + Map adapters = activeJsonAdapterClasses.get(); + typeAdapter = adapters.get(value); + if (typeAdapter == null) { + if (isTypeAdapter) { + Class> typeAdapterClass = (Class>) value; + typeAdapter = constructorConstructor.get(TypeToken.get(typeAdapterClass)).construct(); + } else if (isJsonSerializer || isJsonDeserializer) { + JsonSerializer serializer = null; + if (isJsonSerializer) { + Class> serializerClass = (Class>) value; + serializer = constructorConstructor.get(TypeToken.get(serializerClass)).construct(); + } + JsonDeserializer deserializer = null; + if (isJsonDeserializer) { + Class> deserializerClass = (Class>) value; + deserializer = constructorConstructor.get(TypeToken.get(deserializerClass)).construct(); + } + typeAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, null); + } + adapters.put(value, typeAdapter); + } } else if (TypeAdapterFactory.class.isAssignableFrom(value)) { - Class typeAdapterFactory = (Class) value; - typeAdapter = constructorConstructor.get(TypeToken.get(typeAdapterFactory)) - .construct() - .create(gson, fieldType); + TypeAdapterFactory factory = getTypeAdapterFactory(annotation, constructorConstructor); + typeAdapter = factory == null ? null : factory.create(gson, type); } else { throw new IllegalArgumentException( - "@JsonAdapter value must be TypeAdapter or TypeAdapterFactory reference."); + "@JsonAdapter value must be TypeAdapter, TypeAdapterFactory, JsonSerializer or JsonDeserializer reference."); } if (typeAdapter != null) { typeAdapter = typeAdapter.nullSafe(); } return typeAdapter; } + + + @SuppressWarnings({ "unchecked", "rawtypes" }) // Casts guarded by conditionals. + TypeAdapterFactory getTypeAdapterFactory(JsonAdapter annotation, ConstructorConstructor constructorConstructor) { + Class value = annotation.value(); + if (!TypeAdapterFactory.class.isAssignableFrom(value)) return null; + Map adapterFactories = activeJsonAdapterFactories.get(); + TypeAdapterFactory factory = adapterFactories.get(value); + if (factory == null) { + Class typeAdapterFactoryClass = (Class) value; + factory = constructorConstructor.get(TypeToken.get(typeAdapterFactoryClass)) + .construct(); + adapterFactories.put(value, factory); + } + return factory; + } } diff --git a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java index 9d9f6be8..45d11224 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java @@ -42,8 +42,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import static com.google.gson.internal.bind.JsonAdapterAnnotationTypeAdapterFactory.getTypeAdapter; - /** * Type adapter that reflects over the fields and methods of a class. */ @@ -51,12 +49,15 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory { private final ConstructorConstructor constructorConstructor; private final FieldNamingStrategy fieldNamingPolicy; private final Excluder excluder; + private final JsonAdapterAnnotationTypeAdapterFactory jsonAdapterFactory; public ReflectiveTypeAdapterFactory(ConstructorConstructor constructorConstructor, - FieldNamingStrategy fieldNamingPolicy, Excluder excluder) { + FieldNamingStrategy fieldNamingPolicy, Excluder excluder, + JsonAdapterAnnotationTypeAdapterFactory jsonAdapterFactory) { this.constructorConstructor = constructorConstructor; this.fieldNamingPolicy = fieldNamingPolicy; this.excluder = excluder; + this.jsonAdapterFactory = jsonAdapterFactory; } public boolean excludeField(Field f, boolean serialize) { @@ -108,7 +109,8 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory { JsonAdapter annotation = field.getAnnotation(JsonAdapter.class); TypeAdapter mapped = null; if (annotation != null) { - mapped = getTypeAdapter(constructorConstructor, context, fieldType, annotation); + mapped = jsonAdapterFactory.getTypeAdapter( + constructorConstructor, context, fieldType, annotation); } final boolean jsonAdapterPresent = mapped != null; if (mapped == null) mapped = context.getAdapter(fieldType); diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java index 6f4a0ce9..539b8353 100644 --- a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java @@ -16,6 +16,10 @@ package com.google.gson.functional; +import java.io.IOException; +import java.util.Arrays; +import java.util.List; + import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.TypeAdapter; @@ -24,7 +28,7 @@ import com.google.gson.annotations.JsonAdapter; import com.google.gson.reflect.TypeToken; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonWriter; -import java.io.IOException; + import junit.framework.TestCase; /** @@ -268,4 +272,35 @@ public final class JsonAdapterAnnotationOnFieldsTest extends TestCase { + " annotated with @JsonAdapter(LongToStringTypeAdapterFactory.class)"); } } + + public void testFieldAnnotationWorksForParameterizedType() { + Gson gson = new Gson(); + String json = gson.toJson(new Gizmo2(Arrays.asList(new Part("Part")))); + assertEquals("{\"part\":\"GizmoPartTypeAdapterFactory\"}", json); + Gizmo2 computer = gson.fromJson("{'part':'Part'}", Gizmo2.class); + assertEquals("GizmoPartTypeAdapterFactory", computer.part.get(0).name); + } + + private static final class Gizmo2 { + @JsonAdapter(Gizmo2PartTypeAdapterFactory.class) + List part; + Gizmo2(List part) { + this.part = part; + } + } + + private static class Gizmo2PartTypeAdapterFactory implements TypeAdapterFactory { + @Override public TypeAdapter create(Gson gson, final TypeToken type) { + return new TypeAdapter() { + @Override public void write(JsonWriter out, T value) throws IOException { + out.value("GizmoPartTypeAdapterFactory"); + } + @SuppressWarnings("unchecked") + @Override public T read(JsonReader in) throws IOException { + in.nextString(); + return (T) Arrays.asList(new Part("GizmoPartTypeAdapterFactory")); + } + }; + } + } } diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java new file mode 100644 index 00000000..de9ad294 --- /dev/null +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java @@ -0,0 +1,164 @@ +/* + * Copyright (C) 2008 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.functional; + +import java.lang.reflect.Type; + +import com.google.gson.Gson; +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.annotations.JsonAdapter; + +import junit.framework.TestCase; + +/** + * Functional tests for the {@link JsonAdapter} annotation on fields where the value is of + * type {@link JsonSerializer} or {@link JsonDeserializer}. + */ +public final class JsonAdapterSerializerDeserializerTest extends TestCase { + + public void testJsonSerializerDeserializerBasedJsonAdapterOnFields() { + Gson gson = new Gson(); + String json = gson.toJson(new Computer(new User("Inderjeet Singh"), null, new User("Jesse Wilson"))); + assertEquals("{\"user1\":\"UserSerializer\",\"user3\":\"UserSerializerDeserializer\"}", json); + Computer computer = gson.fromJson("{'user2':'Jesse Wilson','user3':'Jake Wharton'}", Computer.class); + assertEquals("UserSerializer", computer.user2.name); + assertEquals("UserSerializerDeserializer", computer.user3.name); + } + + private static final class Computer { + @JsonAdapter(UserSerializer.class) final User user1; + @JsonAdapter(UserDeserializer.class) final User user2; + @JsonAdapter(UserSerializerDeserializer.class) final User user3; + Computer(User user1, User user2, User user3) { + this.user1 = user1; + this.user2 = user2; + this.user3 = user3; + } + } + + private static final class User { + public final String name; + private User(String name) { + this.name = name; + } + } + + private static final class UserSerializer implements JsonSerializer { + @Override + public JsonElement serialize(User src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("UserSerializer"); + } + } + + private static final class UserDeserializer implements JsonDeserializer { + @Override + public User deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) + throws JsonParseException { + return new User("UserSerializer"); + } + } + + private static final class UserSerializerDeserializer implements JsonSerializer, JsonDeserializer { + @Override + public JsonElement serialize(User src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("UserSerializerDeserializer"); + } + @Override + public User deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) + throws JsonParseException { + return new User("UserSerializerDeserializer"); + } + } + + public void testJsonSerializerDeserializerBasedJsonAdapterOnClass() { + Gson gson = new Gson(); + String json = gson.toJson(new Computer2(new User2("Inderjeet Singh"))); + assertEquals("{\"user\":\"UserSerializerDeserializer2\"}", json); + Computer2 computer = gson.fromJson("{'user':'Inderjeet Singh'}", Computer2.class); + assertEquals("UserSerializerDeserializer2", computer.user.name); + } + + private static final class Computer2 { + final User2 user; + Computer2(User2 user) { + this.user = user; + } + } + + @JsonAdapter(UserSerializerDeserializer2.class) + private static final class User2 { + public final String name; + private User2(String name) { + this.name = name; + } + } + + private static final class UserSerializerDeserializer2 implements JsonSerializer, JsonDeserializer { + @Override + public JsonElement serialize(User2 src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("UserSerializerDeserializer2"); + } + @Override + public User2 deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) + throws JsonParseException { + return new User2("UserSerializerDeserializer2"); + } + } + + public void testDifferentJsonAdaptersForGenericFieldsOfSameRawType() { + Container c = new Container("Foo", 10); + Gson gson = new Gson(); + String json = gson.toJson(c); + assertTrue(json.contains("\"a\":\"BaseStringAdapter\"")); + assertTrue(json.contains("\"b\":\"BaseIntegerAdapter\"")); + } + + private static final class Container { + @JsonAdapter(BaseStringAdapter.class) Base a; + @JsonAdapter(BaseIntegerAdapter.class) Base b; + Container(String a, int b) { + this.a = new Base(a); + this.b = new Base(b); + } + } + + private static final class Base { + @SuppressWarnings("unused") + T value; + Base(T value) { + this.value = value; + } + } + + private static final class BaseStringAdapter implements JsonSerializer> { + @Override public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("BaseStringAdapter"); + } + } + + private static final class BaseIntegerAdapter implements JsonSerializer> { + @Override public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("BaseIntegerAdapter"); + } + } +} diff --git a/gson/src/test/java/com/google/gson/functional/RuntimeTypeAdapterFactoryFunctionalTest.java b/gson/src/test/java/com/google/gson/functional/RuntimeTypeAdapterFactoryFunctionalTest.java index 7959fc86..1284e2c9 100644 --- a/gson/src/test/java/com/google/gson/functional/RuntimeTypeAdapterFactoryFunctionalTest.java +++ b/gson/src/test/java/com/google/gson/functional/RuntimeTypeAdapterFactoryFunctionalTest.java @@ -45,7 +45,7 @@ public final class RuntimeTypeAdapterFactoryFunctionalTest extends TestCase { * This test also ensures that {@link TypeAdapterFactory} registered through {@link JsonAdapter} * work correctly for {@link Gson#getDelegateAdapter(TypeAdapterFactory, TypeToken)}. */ - public void testSubclassesAutomaticallySerialzed() throws Exception { + public void testSubclassesAutomaticallySerialized() throws Exception { Shape shape = new Circle(25); String json = gson.toJson(shape); shape = gson.fromJson(json, Shape.class); From 943c67427630f3da3f6ad7d8edfdf8b0f5451d9a Mon Sep 17 00:00:00 2001 From: Inderjeet Singh Date: Tue, 31 May 2016 10:14:24 -0700 Subject: [PATCH 2/4] Removed ThreadLocal for activeJsonAdapterClasses --- ...onAdapterAnnotationTypeAdapterFactory.java | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java index 9de6b041..1eaacaa4 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java @@ -36,13 +36,6 @@ import com.google.gson.reflect.TypeToken; */ public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapterFactory { - @SuppressWarnings("rawtypes") - private final ThreadLocal> activeJsonAdapterClasses = new ThreadLocal>() { - @Override protected Map initialValue() { - // No need for a thread-safe map since we are using it in a single thread - return new HashMap(); - } - }; @SuppressWarnings("rawtypes") private final ThreadLocal> activeJsonAdapterFactories = new ThreadLocal>() { @Override protected Map initialValue() { @@ -93,26 +86,23 @@ public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapte TypeAdapter typeAdapter; if (isTypeAdapter || isJsonSerializer || isJsonDeserializer) { - Map adapters = activeJsonAdapterClasses.get(); - typeAdapter = adapters.get(value); - if (typeAdapter == null) { - if (isTypeAdapter) { - Class> typeAdapterClass = (Class>) value; - typeAdapter = constructorConstructor.get(TypeToken.get(typeAdapterClass)).construct(); - } else if (isJsonSerializer || isJsonDeserializer) { - JsonSerializer serializer = null; - if (isJsonSerializer) { - Class> serializerClass = (Class>) value; - serializer = constructorConstructor.get(TypeToken.get(serializerClass)).construct(); - } - JsonDeserializer deserializer = null; - if (isJsonDeserializer) { - Class> deserializerClass = (Class>) value; - deserializer = constructorConstructor.get(TypeToken.get(deserializerClass)).construct(); - } - typeAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, null); + if (isTypeAdapter) { + Class> typeAdapterClass = (Class>) value; + typeAdapter = constructorConstructor.get(TypeToken.get(typeAdapterClass)).construct(); + } else if (isJsonSerializer || isJsonDeserializer) { + JsonSerializer serializer = null; + if (isJsonSerializer) { + Class> serializerClass = (Class>) value; + serializer = constructorConstructor.get(TypeToken.get(serializerClass)).construct(); } - adapters.put(value, typeAdapter); + JsonDeserializer deserializer = null; + if (isJsonDeserializer) { + Class> deserializerClass = (Class>) value; + deserializer = constructorConstructor.get(TypeToken.get(deserializerClass)).construct(); + } + typeAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, null); + } else { + typeAdapter = null; } } else if (TypeAdapterFactory.class.isAssignableFrom(value)) { TypeAdapterFactory factory = getTypeAdapterFactory(annotation, constructorConstructor); From 2df65502edc90a04ad0cf12319aee0788972d5cb Mon Sep 17 00:00:00 2001 From: jwilson Date: Thu, 2 Jun 2016 00:18:03 -0400 Subject: [PATCH 3/4] Don't use ThreadLocals for @JsonAdapter factories and getDelegateAdapter(). --- gson/src/main/java/com/google/gson/Gson.java | 20 +++-- ...onAdapterAnnotationTypeAdapterFactory.java | 87 ++++--------------- .../bind/ReflectiveTypeAdapterFactory.java | 3 +- ...JsonAdapterSerializerDeserializerTest.java | 2 +- 4 files changed, 30 insertions(+), 82 deletions(-) diff --git a/gson/src/main/java/com/google/gson/Gson.java b/gson/src/main/java/com/google/gson/Gson.java index c34c896d..67ae06e0 100644 --- a/gson/src/main/java/com/google/gson/Gson.java +++ b/gson/src/main/java/com/google/gson/Gson.java @@ -488,26 +488,28 @@ public final class Gson { * @since 2.2 */ public TypeAdapter getDelegateAdapter(TypeAdapterFactory skipPast, TypeToken type) { - // If the specified skipPast factory is not registered, ignore it. - boolean skipPastFound = skipPast == null - || (!factories.contains(skipPast) && jsonAdapterFactory.getDelegateAdapterFactory(type) == 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)) { + skipPast = jsonAdapterFactory; + } for (TypeAdapterFactory factory : factories) { if (!skipPastFound) { - skipPastFound = factory == skipPast; - if (!skipPastFound && factory instanceof JsonAdapterAnnotationTypeAdapterFactory) { - // Also check if there is a registered JsonAdapter for it - factory = ((JsonAdapterAnnotationTypeAdapterFactory)factory).getDelegateAdapterFactory(type); - skipPastFound = factory == skipPast; + if (factory == skipPast) { + skipPastFound = true; } continue; } + TypeAdapter candidate = factory.create(this, type); if (candidate != null) { return candidate; } } - throw new IllegalArgumentException("GSON cannot serialize or deserialize " + type); + throw new IllegalArgumentException("GSON cannot serialize " + type); } /** diff --git a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java index 1eaacaa4..c079b64d 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/JsonAdapterAnnotationTypeAdapterFactory.java @@ -16,9 +16,6 @@ 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; @@ -35,15 +32,6 @@ import com.google.gson.reflect.TypeToken; * @since 2.3 */ public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapterFactory { - - @SuppressWarnings("rawtypes") - private final ThreadLocal> activeJsonAdapterFactories = new ThreadLocal>() { - @Override protected Map initialValue() { - // No need for a thread-safe map since we are using it in a single thread - return new HashMap(); - } - }; - private final ConstructorConstructor constructorConstructor; public JsonAdapterAnnotationTypeAdapterFactory(ConstructorConstructor constructorConstructor) { @@ -61,75 +49,34 @@ public final class JsonAdapterAnnotationTypeAdapterFactory implements TypeAdapte return (TypeAdapter) getTypeAdapter(constructorConstructor, gson, targetType, annotation); } - public TypeAdapter getDelegateAdapter(Gson gson, TypeAdapterFactory skipPast, TypeToken targetType) { - TypeAdapterFactory factory = getDelegateAdapterFactory(targetType); - if (factory == skipPast) factory = null; - return factory == null ? null: factory.create(gson, targetType); - } - - public TypeAdapterFactory getDelegateAdapterFactory(TypeToken targetType) { - Class annotatedClass = targetType.getRawType(); - JsonAdapter annotation = annotatedClass.getAnnotation(JsonAdapter.class); - if (annotation == null) { - return null; - } - return getTypeAdapterFactory(annotation, constructorConstructor); - } - @SuppressWarnings({ "unchecked", "rawtypes" }) // Casts guarded by conditionals. TypeAdapter getTypeAdapter(ConstructorConstructor constructorConstructor, Gson gson, TypeToken type, JsonAdapter annotation) { - Class value = annotation.value(); - boolean isTypeAdapter = TypeAdapter.class.isAssignableFrom(value); - boolean isJsonSerializer = JsonSerializer.class.isAssignableFrom(value); - boolean isJsonDeserializer = JsonDeserializer.class.isAssignableFrom(value); + Object instance = constructorConstructor.get(TypeToken.get(annotation.value())).construct(); TypeAdapter typeAdapter; - if (isTypeAdapter || isJsonSerializer || isJsonDeserializer) { - if (isTypeAdapter) { - Class> typeAdapterClass = (Class>) value; - typeAdapter = constructorConstructor.get(TypeToken.get(typeAdapterClass)).construct(); - } else if (isJsonSerializer || isJsonDeserializer) { - JsonSerializer serializer = null; - if (isJsonSerializer) { - Class> serializerClass = (Class>) value; - serializer = constructorConstructor.get(TypeToken.get(serializerClass)).construct(); - } - JsonDeserializer deserializer = null; - if (isJsonDeserializer) { - Class> deserializerClass = (Class>) value; - deserializer = constructorConstructor.get(TypeToken.get(deserializerClass)).construct(); - } - typeAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, null); - } else { - typeAdapter = null; - } - } else if (TypeAdapterFactory.class.isAssignableFrom(value)) { - TypeAdapterFactory factory = getTypeAdapterFactory(annotation, constructorConstructor); - typeAdapter = factory == null ? null : factory.create(gson, type); + if (instance instanceof TypeAdapter) { + typeAdapter = (TypeAdapter) instance; + } else if (instance instanceof TypeAdapterFactory) { + typeAdapter = ((TypeAdapterFactory) instance).create(gson, type); + } else if (instance instanceof JsonSerializer || instance instanceof JsonDeserializer) { + JsonSerializer serializer = instance instanceof JsonSerializer + ? (JsonSerializer) instance + : null; + JsonDeserializer deserializer = instance instanceof JsonDeserializer + ? (JsonDeserializer) instance + : null; + typeAdapter = new TreeTypeAdapter(serializer, deserializer, gson, type, null); } else { throw new IllegalArgumentException( - "@JsonAdapter value must be TypeAdapter, TypeAdapterFactory, JsonSerializer or JsonDeserializer reference."); + "@JsonAdapter value must be TypeAdapter, TypeAdapterFactory, " + + "JsonSerializer or JsonDeserializer reference."); } + if (typeAdapter != null) { typeAdapter = typeAdapter.nullSafe(); } + return typeAdapter; } - - - @SuppressWarnings({ "unchecked", "rawtypes" }) // Casts guarded by conditionals. - TypeAdapterFactory getTypeAdapterFactory(JsonAdapter annotation, ConstructorConstructor constructorConstructor) { - Class value = annotation.value(); - if (!TypeAdapterFactory.class.isAssignableFrom(value)) return null; - Map adapterFactories = activeJsonAdapterFactories.get(); - TypeAdapterFactory factory = adapterFactories.get(value); - if (factory == null) { - Class typeAdapterFactoryClass = (Class) value; - factory = constructorConstructor.get(TypeToken.get(typeAdapterFactoryClass)) - .construct(); - adapterFactories.put(value, factory); - } - return factory; - } } diff --git a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java index 45d11224..34e97664 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java @@ -32,7 +32,6 @@ import com.google.gson.reflect.TypeToken; import com.google.gson.stream.JsonReader; import com.google.gson.stream.JsonToken; import com.google.gson.stream.JsonWriter; - import java.io.IOException; import java.lang.reflect.Field; import java.lang.reflect.Type; @@ -110,7 +109,7 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory { TypeAdapter mapped = null; if (annotation != null) { mapped = jsonAdapterFactory.getTypeAdapter( - constructorConstructor, context, fieldType, annotation); + constructorConstructor, context, fieldType, annotation); } final boolean jsonAdapterPresent = mapped != null; if (mapped == null) mapped = context.getAdapter(fieldType); diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java index de9ad294..8ab4e128 100644 --- a/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterSerializerDeserializerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2008 Google Inc. + * 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. From 1f859ec769ed3a220bf8adf2423ba29b44db94e2 Mon Sep 17 00:00:00 2001 From: Inderjeet Singh Date: Tue, 14 Jun 2016 16:34:34 -0700 Subject: [PATCH 4/4] addressed code review comments. --- gson/src/main/java/com/google/gson/Gson.java | 5 ++--- .../JsonAdapterAnnotationOnFieldsTest.java | 22 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/gson/src/main/java/com/google/gson/Gson.java b/gson/src/main/java/com/google/gson/Gson.java index 67ae06e0..e97b1627 100644 --- a/gson/src/main/java/com/google/gson/Gson.java +++ b/gson/src/main/java/com/google/gson/Gson.java @@ -134,7 +134,7 @@ public final class Gson { private final boolean generateNonExecutableJson; private final boolean prettyPrinting; private final boolean lenient; - private JsonAdapterAnnotationTypeAdapterFactory jsonAdapterFactory; + private final JsonAdapterAnnotationTypeAdapterFactory jsonAdapterFactory; /** * Constructs a Gson object with default configuration. The default configuration has the @@ -488,14 +488,13 @@ public final class Gson { * @since 2.2 */ public TypeAdapter getDelegateAdapter(TypeAdapterFactory skipPast, TypeToken type) { - 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)) { skipPast = jsonAdapterFactory; } + boolean skipPastFound = false; for (TypeAdapterFactory factory : factories) { if (!skipPastFound) { if (factory == skipPast) { diff --git a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java index 539b8353..706fe60f 100644 --- a/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java +++ b/gson/src/test/java/com/google/gson/functional/JsonAdapterAnnotationOnFieldsTest.java @@ -274,20 +274,20 @@ public final class JsonAdapterAnnotationOnFieldsTest extends TestCase { } public void testFieldAnnotationWorksForParameterizedType() { - Gson gson = new Gson(); - String json = gson.toJson(new Gizmo2(Arrays.asList(new Part("Part")))); - assertEquals("{\"part\":\"GizmoPartTypeAdapterFactory\"}", json); - Gizmo2 computer = gson.fromJson("{'part':'Part'}", Gizmo2.class); - assertEquals("GizmoPartTypeAdapterFactory", computer.part.get(0).name); - } + Gson gson = new Gson(); + String json = gson.toJson(new Gizmo2(Arrays.asList(new Part("Part")))); + assertEquals("{\"part\":\"GizmoPartTypeAdapterFactory\"}", json); + Gizmo2 computer = gson.fromJson("{'part':'Part'}", Gizmo2.class); + assertEquals("GizmoPartTypeAdapterFactory", computer.part.get(0).name); + } private static final class Gizmo2 { - @JsonAdapter(Gizmo2PartTypeAdapterFactory.class) - List part; - Gizmo2(List part) { - this.part = part; - } + @JsonAdapter(Gizmo2PartTypeAdapterFactory.class) + List part; + Gizmo2(List part) { + this.part = part; } + } private static class Gizmo2PartTypeAdapterFactory implements TypeAdapterFactory { @Override public TypeAdapter create(Gson gson, final TypeToken type) {