Fix changes to GsonBuilder affecting existing Gson instances (#1815)

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.
google · Aug 22, 2022 · 5bebf97 · 5bebf97
1 parent 517d3b1
commit 5bebf97
Show file tree

Hide file tree

Showing 2 changed files with 258 additions and 20 deletions.
diff --git a/gson/src/test/java/com/google/gson/GsonBuilderTest.java b/gson/src/test/java/com/google/gson/GsonBuilderTest.java
@@ -16,14 +16,14 @@
 
 package com.google.gson;
 
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonWriter;
+import java.io.IOException;
+import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
-
 import junit.framework.TestCase;
 
-import com.google.gson.stream.JsonReader;
-import com.google.gson.stream.JsonWriter;
-
 /**
  * Unit tests for {@link GsonBuilder}.
  *
@@ -41,8 +41,99 @@ public class GsonBuilderTest extends TestCase {
 
   public void testCreatingMoreThanOnce() {
     GsonBuilder builder = new GsonBuilder();
-    builder.create();
-    builder.create();
+    Gson gson = builder.create();
+    assertNotNull(gson);
+    assertNotNull(builder.create());
+
+    builder.setFieldNamingStrategy(new FieldNamingStrategy() {
+      @Override public String translateName(Field f) {
+        return "test";
+      }
+    });
+
+    Gson otherGson = builder.create();
+    assertNotNull(otherGson);
+    // Should be different instances because builder has been modified in the meantime
+    assertNotSame(gson, otherGson);
+  }
+
+  /**
+   * 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<CustomClass1>() {
+      @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<CustomClass2>() {
+      @Override public JsonElement serialize(CustomClass2 src, Type typeOfSrc, JsonSerializationContext context) {
+        return new JsonPrimitive("custom-hierarchy-adapter");
+      }
+    });
+    gsonBuilder.registerTypeAdapter(CustomClass3.class, new InstanceCreator<CustomClass3>() {
+      @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() {
@@ -52,20 +143,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";
@@ -85,6 +162,20 @@ 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);
+    }
+  }
+
   public void testDisableJdkUnsafe() {
     Gson gson = new GsonBuilder()
         .disableJdkUnsafe()

diff --git a/gson/src/test/java/com/google/gson/GsonTest.java b/gson/src/test/java/com/google/gson/GsonTest.java
@@ -221,4 +221,151 @@ public void testNewJsonReader_Custom() throws IOException {
     assertEquals("test", jsonReader.nextString());
     jsonReader.close();
   }
+
+  /**
+   * 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<CustomClass1>() {
+      @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<CustomClass2>() {
+      @Override public JsonElement serialize(CustomClass2 src, Type typeOfSrc, JsonSerializationContext context) {
+        return new JsonPrimitive("custom-hierarchy-adapter");
+      }
+    });
+    gsonBuilder.registerTypeAdapter(CustomClass3.class, new InstanceCreator<CustomClass3>() {
+      @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<CustomClass1>() {
+        @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<CustomClass2>() {
+        @Override public JsonElement serialize(CustomClass2 src, Type typeOfSrc, JsonSerializationContext context) {
+          return new JsonPrimitive("custom-hierarchy-adapter");
+        }
+      })
+      .registerTypeAdapter(CustomClass3.class, new InstanceCreator<CustomClass3>() {
+        @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<CustomClass1>() {
+      @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<CustomClass2>() {
+      @Override public JsonElement serialize(CustomClass2 src, Type typeOfSrc, JsonSerializationContext context) {
+        return new JsonPrimitive("overwritten custom-hierarchy-adapter");
+      }
+    });
+    gsonBuilder.registerTypeAdapter(CustomClass3.class, new InstanceCreator<CustomClass3>() {
+      @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);
+    }
+  }
 }