From e7566085683827ae9278880c38e21a80ccd573d9 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Fri, 9 Sep 2011 06:26:21 +0000 Subject: [PATCH] Forbid custom serializers for primitive types (so we can avoid boxing in the reflective and array adapters) --- gson/GSON 2.0 NOTES.txt | 8 +- .../java/com/google/gson/GsonBuilder.java | 5 +- .../main/java/com/google/gson/Primitives.java | 5 +- .../functional/CustomDeserializerTest.java | 68 +++------ .../functional/CustomTypeAdaptersTest.java | 141 +++++++----------- 5 files changed, 88 insertions(+), 139 deletions(-) diff --git a/gson/GSON 2.0 NOTES.txt b/gson/GSON 2.0 NOTES.txt index 43eeb8fe..e26aba83 100644 --- a/gson/GSON 2.0 NOTES.txt +++ b/gson/GSON 2.0 NOTES.txt @@ -30,4 +30,10 @@ com.google.gson.functional.PrimitiveTest.testDeserializingBigIntegerAsLong GSON 1.x uses arbitrary precision for primitive type conversion (so -122.08e-2132 != 0) GSON 2.x uses double precision (so -122.08e-2132 == 0) -com.google.gson.functional.PrimitiveTest.testDeserializingBigDecimalAsLongFails \ No newline at end of file +com.google.gson.functional.PrimitiveTest.testDeserializingBigDecimalAsLongFails + + +GSON 1.x supports type adapters for primitive types +GSON 2.x doesn't +com.google.gson.functional.CustomTypeAdaptersTest.testCustomSerializerForLong +com.google.gson.functional.CustomTypeAdaptersTest.testCustomDeserializerForLong diff --git a/gson/src/main/java/com/google/gson/GsonBuilder.java b/gson/src/main/java/com/google/gson/GsonBuilder.java index bd707044..cdd123cc 100644 --- a/gson/src/main/java/com/google/gson/GsonBuilder.java +++ b/gson/src/main/java/com/google/gson/GsonBuilder.java @@ -18,7 +18,6 @@ package com.google.gson; import com.google.gson.DefaultTypeAdapters.DefaultDateTypeAdapter; import com.google.gson.internal.$Gson$Preconditions; - import java.lang.reflect.Type; import java.sql.Timestamp; import java.text.DateFormat; @@ -495,6 +494,10 @@ public final class GsonBuilder { private GsonBuilder registerTypeAdapter(Type type, Object typeAdapter, boolean isSystem) { $Gson$Preconditions.checkArgument(typeAdapter instanceof JsonSerializer || typeAdapter instanceof JsonDeserializer || typeAdapter instanceof InstanceCreator); + if (Primitives.isPrimitive(type) || Primitives.isWrapperType(type)) { + throw new IllegalArgumentException( + "Cannot register type adapters for " + type); + } if (typeAdapter instanceof InstanceCreator) { registerInstanceCreator(type, (InstanceCreator) typeAdapter, isSystem); } diff --git a/gson/src/main/java/com/google/gson/Primitives.java b/gson/src/main/java/com/google/gson/Primitives.java index 7d43f7de..446890d4 100644 --- a/gson/src/main/java/com/google/gson/Primitives.java +++ b/gson/src/main/java/com/google/gson/Primitives.java @@ -17,13 +17,12 @@ package com.google.gson; +import com.google.gson.internal.$Gson$Preconditions; import java.lang.reflect.Type; import java.util.Collections; import java.util.HashMap; import java.util.Map; -import com.google.gson.internal.$Gson$Preconditions; - /** * Contains static utility methods pertaining to primitive types and their * corresponding wrapper types. @@ -78,7 +77,7 @@ final class Primitives { * * @see Class#isPrimitive */ - public static boolean isWrapperType(Class type) { + public static boolean isWrapperType(Type type) { return WRAPPER_TO_PRIMITIVE_TYPE.containsKey( $Gson$Preconditions.checkNotNull(type)); } diff --git a/gson/src/test/java/com/google/gson/functional/CustomDeserializerTest.java b/gson/src/test/java/com/google/gson/functional/CustomDeserializerTest.java index 21354d07..54ecade2 100644 --- a/gson/src/test/java/com/google/gson/functional/CustomDeserializerTest.java +++ b/gson/src/test/java/com/google/gson/functional/CustomDeserializerTest.java @@ -31,7 +31,7 @@ import junit.framework.TestCase; import java.lang.reflect.Type; /** - * Functional Test exercising custom deserialization only. When test applies to both + * Functional Test exercising custom deserialization only. When test applies to both * serialization and deserialization then add it to CustomTypeAdapterTest. * * @author Joel Leitch @@ -39,27 +39,27 @@ import java.lang.reflect.Type; public class CustomDeserializerTest extends TestCase { private static final String DEFAULT_VALUE = "test123"; private static final String SUFFIX = "blah"; - + private Gson gson; - + @Override protected void setUp() throws Exception { super.setUp(); gson = new GsonBuilder().registerTypeAdapter(DataHolder.class, new DataHolderDeserializer()).create(); } - + public void testDefaultConstructorNotCalledOnObject() throws Exception { DataHolder data = new DataHolder(DEFAULT_VALUE); String json = gson.toJson(data); - + DataHolder actual = gson.fromJson(json, DataHolder.class); assertEquals(DEFAULT_VALUE + SUFFIX, actual.getData()); } - + public void testDefaultConstructorNotCalledOnField() throws Exception { DataHolderWrapper dataWrapper = new DataHolderWrapper(new DataHolder(DEFAULT_VALUE)); String json = gson.toJson(dataWrapper); - + DataHolderWrapper actual = gson.fromJson(json, DataHolderWrapper.class); assertEquals(DEFAULT_VALUE + SUFFIX, actual.getWrappedData().getData()); } @@ -72,25 +72,25 @@ public class CustomDeserializerTest extends TestCase { private DataHolder() { throw new IllegalStateException(); } - + public DataHolder(String data) { this.data = data; } - + public String getData() { return data; } } - + private static class DataHolderWrapper { private final DataHolder wrappedData; - + // For use by Gson @SuppressWarnings("unused") private DataHolderWrapper() { this(new DataHolder(DEFAULT_VALUE)); } - + public DataHolderWrapper(DataHolder data) { this.wrappedData = data; } @@ -99,7 +99,7 @@ public class CustomDeserializerTest extends TestCase { return wrappedData; } } - + private static class DataHolderDeserializer implements JsonDeserializer { public DataHolder deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException { @@ -108,7 +108,7 @@ public class CustomDeserializerTest extends TestCase { return new DataHolder(dataString + SUFFIX); } } - + public void testJsonTypeFieldBasedDeserialization() { String json = "{field1:'abc',field2:'def',__type__:'SUB_TYPE1'}"; Gson gson = new GsonBuilder().registerTypeAdapter(MyBase.class, new JsonDeserializer() { @@ -117,16 +117,16 @@ public class CustomDeserializerTest extends TestCase { String type = json.getAsJsonObject().get(MyBase.TYPE_ACCESS).getAsString(); return context.deserialize(json, SubTypes.valueOf(type).getSubclass()); } - }).create(); + }).create(); SubType1 target = (SubType1) gson.fromJson(json, MyBase.class); - assertEquals("abc", target.field1); + assertEquals("abc", target.field1); } private static class MyBase { static final String TYPE_ACCESS = "__type__"; } - private enum SubTypes { + private enum SubTypes { SUB_TYPE1(SubType1.class), SUB_TYPE2(SubType2.class); private final Type subClass; @@ -139,14 +139,14 @@ public class CustomDeserializerTest extends TestCase { } private static class SubType1 extends MyBase { - String field1; + String field1; } private static class SubType2 extends MyBase { @SuppressWarnings("unused") - String field2; + String field2; } - + public void testCustomDeserializerReturnsNullForTopLevelObject() { Gson gson = new GsonBuilder() .registerTypeAdapter(Base.class, new JsonDeserializer() { @@ -173,34 +173,6 @@ public class CustomDeserializerTest extends TestCase { assertNull(target.base); } - public void testCustomDeserializerReturnsNullForTopLevelPrimitives() { - Gson gson = new GsonBuilder() - .registerTypeAdapter(long.class, new JsonDeserializer() { - public Long deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) - throws JsonParseException { - return null; - } - }).create(); - String json = "10"; - assertNull(gson.fromJson(json, long.class)); - } - - public void testCustomDeserializerReturnsNullForPrimitiveFields() { - Gson gson = new GsonBuilder() - .registerTypeAdapter(long.class, new JsonDeserializer() { - public Long deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) - throws JsonParseException { - return null; - } - }).create(); - String json = "{field:10}"; - ClassWithLong target = gson.fromJson(json, ClassWithLong.class); - assertEquals(0, target.field); - } - private static class ClassWithLong { - long field; - } - public void testCustomDeserializerReturnsNullForArrayElements() { Gson gson = new GsonBuilder() .registerTypeAdapter(Base.class, new JsonDeserializer() { diff --git a/gson/src/test/java/com/google/gson/functional/CustomTypeAdaptersTest.java b/gson/src/test/java/com/google/gson/functional/CustomTypeAdaptersTest.java index 6af69791..9343e6cc 100644 --- a/gson/src/test/java/com/google/gson/functional/CustomTypeAdaptersTest.java +++ b/gson/src/test/java/com/google/gson/functional/CustomTypeAdaptersTest.java @@ -83,14 +83,14 @@ public class CustomTypeAdaptersTest extends TestCase { ClassWithCustomTypeConverter target = gson.fromJson(json, ClassWithCustomTypeConverter.class); assertEquals(5, target.getBag().getIntValue()); } - + public void disable_testCustomSerializersOfSelf() { Gson gson = createGsonObjectWithFooTypeAdapter(); Gson basicGson = new Gson(); Foo newFooObject = new Foo(1, 2L); String jsonFromCustomSerializer = gson.toJson(newFooObject); String jsonFromGson = basicGson.toJson(newFooObject); - + assertEquals(jsonFromGson, jsonFromCustomSerializer); } @@ -100,7 +100,7 @@ public class CustomTypeAdaptersTest extends TestCase { Foo expectedFoo = new Foo(1, 2L); String json = basicGson.toJson(expectedFoo); Foo newFooObject = gson.fromJson(json, Foo.class); - + assertEquals(expectedFoo.key, newFooObject.key); assertEquals(expectedFoo.value, newFooObject.value); } @@ -130,58 +130,58 @@ public class CustomTypeAdaptersTest extends TestCase { ClassWithCustomTypeConverter target = gson.fromJson(json, ClassWithCustomTypeConverter.class); assertEquals(7, target.getBag().getIntValue()); } - + public void testCustomTypeAdapterDoesNotAppliesToSubClasses() { Gson gson = new GsonBuilder().registerTypeAdapter(Base.class, new JsonSerializer () { public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) { JsonObject json = new JsonObject(); json.addProperty("value", src.baseValue); return json; - } + } }).create(); Base b = new Base(); String json = gson.toJson(b); - assertTrue(json.contains("value")); + assertTrue(json.contains("value")); b = new Derived(); json = gson.toJson(b); - assertTrue(json.contains("derivedValue")); + assertTrue(json.contains("derivedValue")); } - + public void testCustomTypeAdapterAppliesToSubClassesSerializedAsBaseClass() { Gson gson = new GsonBuilder().registerTypeAdapter(Base.class, new JsonSerializer () { public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) { JsonObject json = new JsonObject(); json.addProperty("value", src.baseValue); return json; - } + } }).create(); Base b = new Base(); String json = gson.toJson(b); - assertTrue(json.contains("value")); + assertTrue(json.contains("value")); b = new Derived(); json = gson.toJson(b, Base.class); - assertTrue(json.contains("value")); + assertTrue(json.contains("value")); assertFalse(json.contains("derivedValue")); } - + private static class Base { int baseValue = 2; } - + private static class Derived extends Base { @SuppressWarnings("unused") int derivedValue = 3; } - - + + private Gson createGsonObjectWithFooTypeAdapter() { return new GsonBuilder().registerTypeAdapter(Foo.class, new FooTypeAdapter()).create(); } - + public static class Foo { private final int key; private final long value; - + public Foo() { this(0, 0L); } @@ -191,7 +191,7 @@ public class CustomTypeAdaptersTest extends TestCase { this.value = value; } } - + public static class FooTypeAdapter implements JsonSerializer, JsonDeserializer { public Foo deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException { @@ -202,62 +202,31 @@ public class CustomTypeAdaptersTest extends TestCase { return context.serialize(src, typeOfSrc); } } - - public void testCustomSerializerForLong() { - final ClassWithBooleanField customSerializerInvoked = new ClassWithBooleanField(); - customSerializerInvoked.value = false; - Gson gson = new GsonBuilder().registerTypeAdapter(Long.class, new JsonSerializer() { - public JsonElement serialize(Long src, Type typeOfSrc, JsonSerializationContext context) { - customSerializerInvoked.value = true; - return new JsonPrimitive(src); - } - }).serializeNulls().create(); - ClassWithWrapperLongField src = new ClassWithWrapperLongField(); - String json = gson.toJson(src); - assertTrue(json.contains("\"value\":null")); - assertFalse(customSerializerInvoked.value); - - customSerializerInvoked.value = false; - src.value = 10L; - json = gson.toJson(src); - assertTrue(json.contains("\"value\":10")); - assertTrue(customSerializerInvoked.value); - } - - public void testCustomDeserializerForLong() { - final ClassWithBooleanField customDeserializerInvoked = new ClassWithBooleanField(); - customDeserializerInvoked.value = false; - Gson gson = new GsonBuilder().registerTypeAdapter(Long.class, new JsonDeserializer() { - public Long deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) - throws JsonParseException { - customDeserializerInvoked.value = true; - if (json == null || json.isJsonNull()) { - return null; + + public void testCustomSerializerForbiddenForPrimitives() { + try { + new GsonBuilder().registerTypeAdapter(long.class, new JsonSerializer() { + public JsonElement serialize(Long s, Type t, JsonSerializationContext c) { + throw new AssertionError(); } - Number number = json.getAsJsonPrimitive().getAsNumber(); - return number == null ? null : number.longValue(); - } - }).create(); - String json = "{'value':null}"; - ClassWithWrapperLongField target = gson.fromJson(json, ClassWithWrapperLongField.class); - assertNull(target.value); - assertFalse(customDeserializerInvoked.value); - - customDeserializerInvoked.value = false; - json = "{'value':10}"; - target = gson.fromJson(json, ClassWithWrapperLongField.class); - assertEquals(10L, target.value.longValue()); - assertTrue(customDeserializerInvoked.value); + }); + fail(); + } catch (IllegalArgumentException expected) { + } } - - private static class ClassWithWrapperLongField { - Long value; + + public void testCustomDeserializerForbiddenForPrimitives() { + try { + new GsonBuilder().registerTypeAdapter(long.class, new JsonDeserializer() { + public Long deserialize(JsonElement json, Type t, JsonDeserializationContext c) { + throw new AssertionError(); + } + }); + fail(); + } catch (Exception expected) { + } } - - private static class ClassWithBooleanField { - Boolean value; - } - + public void testCustomByteArraySerializer() { Gson gson = new GsonBuilder().registerTypeAdapter(byte[].class, new JsonSerializer() { public JsonElement serialize(byte[] src, Type typeOfSrc, JsonSerializationContext context) { @@ -266,15 +235,15 @@ public class CustomTypeAdaptersTest extends TestCase { sb.append(b); } return new JsonPrimitive(sb.toString()); - } + } }).create(); byte[] data = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; String json = gson.toJson(data); assertEquals("\"0123456789\"", json); } - + public void testCustomByteArrayDeserializerAndInstanceCreator() { - GsonBuilder gsonBuilder = new GsonBuilder().registerTypeAdapter(byte[].class, + GsonBuilder gsonBuilder = new GsonBuilder().registerTypeAdapter(byte[].class, new JsonDeserializer() { public byte[] deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) throws JsonParseException { @@ -284,7 +253,7 @@ public class CustomTypeAdaptersTest extends TestCase { data[i] = Byte.parseByte(""+str.charAt(i)); } return data; - } + } }); Gson gson = gsonBuilder.create(); String json = "'0123456789'"; @@ -294,7 +263,7 @@ public class CustomTypeAdaptersTest extends TestCase { assertEquals(expected[i], actual[i]); } } - + private static class StringHolder { String part1; String part2; @@ -309,8 +278,8 @@ public class CustomTypeAdaptersTest extends TestCase { this.part2 = part2; } } - - private static class StringHolderTypeAdapter implements JsonSerializer, + + private static class StringHolderTypeAdapter implements JsonSerializer, JsonDeserializer, InstanceCreator { public StringHolder createInstance(Type type) { @@ -318,18 +287,18 @@ public class CustomTypeAdaptersTest extends TestCase { return new StringHolder("unknown:thing"); } - public StringHolder deserialize(JsonElement src, Type type, + public StringHolder deserialize(JsonElement src, Type type, JsonDeserializationContext context) { return new StringHolder(src.getAsString()); } - public JsonElement serialize(StringHolder src, Type typeOfSrc, + public JsonElement serialize(StringHolder src, Type typeOfSrc, JsonSerializationContext context) { String contents = src.part1 + ':' + src.part2; return new JsonPrimitive(contents); } } - + // Test created from Issue 70 public void testCustomAdapterInvokedForCollectionElementSerializationWithType() { Gson gson = new GsonBuilder() @@ -367,7 +336,7 @@ public class CustomTypeAdaptersTest extends TestCase { assertEquals("Jacob", foo.part1); assertEquals("Tomaw", foo.part2); } - + // Test created from Issue 70 public void testCustomAdapterInvokedForMapElementSerializationWithType() { Gson gson = new GsonBuilder() @@ -380,7 +349,7 @@ public class CustomTypeAdaptersTest extends TestCase { String json = gson.toJson(mapOfHolders, mapType); assertTrue(json.contains("\"foo\":\"Jacob:Tomaw\"")); } - + // Test created from Issue 70 public void testCustomAdapterInvokedForMapElementSerialization() { Gson gson = new GsonBuilder() @@ -431,15 +400,15 @@ public class CustomTypeAdaptersTest extends TestCase { this.data = data; } } - + private static class DataHolderWrapper { final DataHolder wrappedData; - + public DataHolderWrapper(DataHolder data) { this.wrappedData = data; } } - + private static class DataHolderSerializer implements JsonSerializer { public JsonElement serialize(DataHolder src, Type typeOfSrc, JsonSerializationContext context) { JsonObject obj = new JsonObject();