From 541f8aba05148bf7d250ce46e8c43d6cb6a57c8b Mon Sep 17 00:00:00 2001 From: Marcono1234 Date: Fri, 6 Nov 2020 15:22:05 +0100 Subject: [PATCH] Fix changes to GsonBuilder affecting existing Gson instances Previously when a GsonBuilder had created a Gson instance and was afterwards reused and different type adapters were added, new GsonBuilder instances obtained from the previous Gson instance through Gson.newBuilder() would have been affected by the GsonBuilder changes. This commit fixes this and additionally adds some more unit tests. --- .../java/com/google/gson/GsonBuilder.java | 7 +- .../java/com/google/gson/GsonBuilderTest.java | 108 +++++++++++-- .../test/java/com/google/gson/GsonTest.java | 147 ++++++++++++++++++ 3 files changed, 247 insertions(+), 15 deletions(-) diff --git a/gson/src/main/java/com/google/gson/GsonBuilder.java b/gson/src/main/java/com/google/gson/GsonBuilder.java index b97be452be..38d0799144 100644 --- a/gson/src/main/java/com/google/gson/GsonBuilder.java +++ b/gson/src/main/java/com/google/gson/GsonBuilder.java @@ -594,12 +594,17 @@ public Gson create() { addTypeAdaptersForDate(datePattern, dateStyle, timeStyle, factories); + // Copy these collections to prevent subsequent modifications of builder from + // affecting Gson instance + Map> instanceCreators = new HashMap>(this.instanceCreators); + List builderFactories = new ArrayList(this.factories); + List builderHierarchyFactories = new ArrayList(this.hierarchyFactories); return new Gson(excluder, fieldNamingPolicy, instanceCreators, serializeNulls, complexMapKeySerialization, generateNonExecutableJson, escapeHtmlChars, prettyPrinting, lenient, serializeSpecialFloatingPointValues, longSerializationPolicy, datePattern, dateStyle, timeStyle, - this.factories, this.hierarchyFactories, factories); + builderFactories, builderHierarchyFactories, factories); } @SuppressWarnings("unchecked") diff --git a/gson/src/test/java/com/google/gson/GsonBuilderTest.java b/gson/src/test/java/com/google/gson/GsonBuilderTest.java index 73601c0e3c..9a104cfc1b 100644 --- a/gson/src/test/java/com/google/gson/GsonBuilderTest.java +++ b/gson/src/test/java/com/google/gson/GsonBuilderTest.java @@ -16,6 +16,7 @@ package com.google.gson; +import java.io.IOException; import java.lang.reflect.Modifier; import java.lang.reflect.Type; @@ -45,6 +46,85 @@ public void testCreatingMoreThanOnce() { builder.create(); } + /** + * Gson instances should not be affected by subsequent modification of GsonBuilder + * which created them. + */ + public void testModificationAfterCreate() { + GsonBuilder gsonBuilder = new GsonBuilder(); + Gson gson = gsonBuilder.create(); + + // Modifications of `gsonBuilder` should not affect `gson` object + gsonBuilder.registerTypeAdapter(CustomClass1.class, new TypeAdapter() { + @Override public CustomClass1 read(JsonReader in) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override public void write(JsonWriter out, CustomClass1 value) throws IOException { + out.value("custom-adapter"); + } + }); + gsonBuilder.registerTypeHierarchyAdapter(CustomClass2.class, new JsonSerializer() { + @Override public JsonElement serialize(CustomClass2 src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("custom-hierarchy-adapter"); + } + }); + gsonBuilder.registerTypeAdapter(CustomClass3.class, new InstanceCreator() { + @Override public CustomClass3 createInstance(Type type) { + return new CustomClass3("custom-instance"); + } + }); + + assertDefaultGson(gson); + // New GsonBuilder created from `gson` should not have been affected by changes + // to `gsonBuilder` either + assertDefaultGson(gson.newBuilder().create()); + + // New Gson instance from modified GsonBuilder should be affected by changes + assertCustomGson(gsonBuilder.create()); + } + + private static void assertDefaultGson(Gson gson) { + // Should use default reflective adapter + String json1 = gson.toJson(new CustomClass1()); + assertEquals("{}", json1); + + // Should use default reflective adapter + String json2 = gson.toJson(new CustomClass2()); + assertEquals("{}", json2); + + // Should use default instance creator + CustomClass3 customClass3 = gson.fromJson("{}", CustomClass3.class); + assertEquals(CustomClass3.NO_ARG_CONSTRUCTOR_VALUE, customClass3.s); + } + + private static void assertCustomGson(Gson gson) { + String json1 = gson.toJson(new CustomClass1()); + assertEquals("\"custom-adapter\"", json1); + + String json2 = gson.toJson(new CustomClass2()); + assertEquals("\"custom-hierarchy-adapter\"", json2); + + CustomClass3 customClass3 = gson.fromJson("{}", CustomClass3.class); + assertEquals("custom-instance", customClass3.s); + } + + static class CustomClass1 { } + static class CustomClass2 { } + static class CustomClass3 { + static final String NO_ARG_CONSTRUCTOR_VALUE = "default instance"; + + final String s; + + public CustomClass3(String s) { + this.s = s; + } + + public CustomClass3() { + this(NO_ARG_CONSTRUCTOR_VALUE); + } + } + public void testExcludeFieldsWithModifiers() { Gson gson = new GsonBuilder() .excludeFieldsWithModifiers(Modifier.VOLATILE, Modifier.PRIVATE) @@ -52,20 +132,6 @@ public void testExcludeFieldsWithModifiers() { assertEquals("{\"d\":\"d\"}", gson.toJson(new HasModifiers())); } - public void testRegisterTypeAdapterForCoreType() { - Type[] types = { - byte.class, - int.class, - double.class, - Short.class, - Long.class, - String.class, - }; - for (Type type : types) { - new GsonBuilder().registerTypeAdapter(type, NULL_TYPE_ADAPTER); - } - } - @SuppressWarnings("unused") static class HasModifiers { private String a = "a"; @@ -84,4 +150,18 @@ public void testTransientFieldExclusion() { static class HasTransients { transient String a = "a"; } + + public void testRegisterTypeAdapterForCoreType() { + Type[] types = { + byte.class, + int.class, + double.class, + Short.class, + Long.class, + String.class, + }; + for (Type type : types) { + new GsonBuilder().registerTypeAdapter(type, NULL_TYPE_ADAPTER); + } + } } diff --git a/gson/src/test/java/com/google/gson/GsonTest.java b/gson/src/test/java/com/google/gson/GsonTest.java index eec2ec91ca..a69b2f4475 100644 --- a/gson/src/test/java/com/google/gson/GsonTest.java +++ b/gson/src/test/java/com/google/gson/GsonTest.java @@ -77,4 +77,151 @@ private static final class TestTypeAdapter extends TypeAdapter { } @Override public Object read(JsonReader in) throws IOException { return null; } } + + /** + * Modifying a GsonBuilder obtained from {@link Gson#newBuilder()} of a + * {@code new Gson()} should not affect the Gson instance it came from. + */ + public void testDefaultGsonNewBuilderModification() { + Gson gson = new Gson(); + GsonBuilder gsonBuilder = gson.newBuilder(); + + // Modifications of `gsonBuilder` should not affect `gson` object + gsonBuilder.registerTypeAdapter(CustomClass1.class, new TypeAdapter() { + @Override public CustomClass1 read(JsonReader in) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override public void write(JsonWriter out, CustomClass1 value) throws IOException { + out.value("custom-adapter"); + } + }); + gsonBuilder.registerTypeHierarchyAdapter(CustomClass2.class, new JsonSerializer() { + @Override public JsonElement serialize(CustomClass2 src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("custom-hierarchy-adapter"); + } + }); + gsonBuilder.registerTypeAdapter(CustomClass3.class, new InstanceCreator() { + @Override public CustomClass3 createInstance(Type type) { + return new CustomClass3("custom-instance"); + } + }); + + assertDefaultGson(gson); + // New GsonBuilder created from `gson` should not have been affected by changes either + assertDefaultGson(gson.newBuilder().create()); + + // But new Gson instance from `gsonBuilder` should use custom adapters + assertCustomGson(gsonBuilder.create()); + } + + private static void assertDefaultGson(Gson gson) { + // Should use default reflective adapter + String json1 = gson.toJson(new CustomClass1()); + assertEquals("{}", json1); + + // Should use default reflective adapter + String json2 = gson.toJson(new CustomClass2()); + assertEquals("{}", json2); + + // Should use default instance creator + CustomClass3 customClass3 = gson.fromJson("{}", CustomClass3.class); + assertEquals(CustomClass3.NO_ARG_CONSTRUCTOR_VALUE, customClass3.s); + } + + /** + * Modifying a GsonBuilder obtained from {@link Gson#newBuilder()} of a custom + * Gson instance (created using a GsonBuilder) should not affect the Gson instance + * it came from. + */ + public void testNewBuilderModification() { + Gson gson = new GsonBuilder() + .registerTypeAdapter(CustomClass1.class, new TypeAdapter() { + @Override public CustomClass1 read(JsonReader in) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override public void write(JsonWriter out, CustomClass1 value) throws IOException { + out.value("custom-adapter"); + } + }) + .registerTypeHierarchyAdapter(CustomClass2.class, new JsonSerializer() { + @Override public JsonElement serialize(CustomClass2 src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("custom-hierarchy-adapter"); + } + }) + .registerTypeAdapter(CustomClass3.class, new InstanceCreator() { + @Override public CustomClass3 createInstance(Type type) { + return new CustomClass3("custom-instance"); + } + }) + .create(); + + assertCustomGson(gson); + + // Modify `gson.newBuilder()` + GsonBuilder gsonBuilder = gson.newBuilder(); + gsonBuilder.registerTypeAdapter(CustomClass1.class, new TypeAdapter() { + @Override public CustomClass1 read(JsonReader in) throws IOException { + throw new UnsupportedOperationException(); + } + + @Override public void write(JsonWriter out, CustomClass1 value) throws IOException { + out.value("overwritten custom-adapter"); + } + }); + gsonBuilder.registerTypeHierarchyAdapter(CustomClass2.class, new JsonSerializer() { + @Override public JsonElement serialize(CustomClass2 src, Type typeOfSrc, JsonSerializationContext context) { + return new JsonPrimitive("overwritten custom-hierarchy-adapter"); + } + }); + gsonBuilder.registerTypeAdapter(CustomClass3.class, new InstanceCreator() { + @Override public CustomClass3 createInstance(Type type) { + return new CustomClass3("overwritten custom-instance"); + } + }); + + // `gson` object should not have been affected by changes to new GsonBuilder + assertCustomGson(gson); + // New GsonBuilder based on `gson` should not have been affected either + assertCustomGson(gson.newBuilder().create()); + + // But new Gson instance from `gsonBuilder` should be affected by changes + Gson otherGson = gsonBuilder.create(); + String json1 = otherGson.toJson(new CustomClass1()); + assertEquals("\"overwritten custom-adapter\"", json1); + + String json2 = otherGson.toJson(new CustomClass2()); + assertEquals("\"overwritten custom-hierarchy-adapter\"", json2); + + CustomClass3 customClass3 = otherGson.fromJson("{}", CustomClass3.class); + assertEquals("overwritten custom-instance", customClass3.s); + } + + private static void assertCustomGson(Gson gson) { + String json1 = gson.toJson(new CustomClass1()); + assertEquals("\"custom-adapter\"", json1); + + String json2 = gson.toJson(new CustomClass2()); + assertEquals("\"custom-hierarchy-adapter\"", json2); + + CustomClass3 customClass3 = gson.fromJson("{}", CustomClass3.class); + assertEquals("custom-instance", customClass3.s); + } + + static class CustomClass1 { } + static class CustomClass2 { } + static class CustomClass3 { + static final String NO_ARG_CONSTRUCTOR_VALUE = "default instance"; + + final String s; + + public CustomClass3(String s) { + this.s = s; + } + + public CustomClass3() { + this(NO_ARG_CONSTRUCTOR_VALUE); + } + } }