Skip to content

Commit

Permalink
Allow registering adapters for JsonElement again (#2789)
Browse files Browse the repository at this point in the history
This is done for backward compatibility, however registering the adapter still
has no effect because the built-in adapter as higher precedence (like it is
the case for multiple years already).

The Javadoc for the GsonBuilder methods has not been adjusted, to discourage
users from trying to register adapters for JsonElement.
  • Loading branch information
Marcono1234 authored Jan 27, 2025
1 parent e5dce84 commit a2b1c3c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 9 deletions.
17 changes: 9 additions & 8 deletions gson/src/main/java/com/google/gson/GsonBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
|| typeAdapter instanceof InstanceCreator<?>
|| typeAdapter instanceof TypeAdapter<?>);

if (isTypeObjectOrJsonElement(type)) {
if (hasNonOverridableAdapter(type)) {
throw new IllegalArgumentException("Cannot override built-in adapter for " + type);
}

Expand All @@ -730,9 +730,14 @@ public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
return this;
}

private static boolean isTypeObjectOrJsonElement(Type type) {
return type instanceof Class
&& (type == Object.class || JsonElement.class.isAssignableFrom((Class<?>) type));
/** Whether the type has a built-in adapter which cannot be overridden. */
private static boolean hasNonOverridableAdapter(Type type) {
return type == Object.class;
// This should also cover `JsonElement.class.isAssignableFrom(type)`, however for backward
// compatibility this is not covered here because really old Gson versions had no built-in
// adapter for JsonElement so users registered custom adapters. These adapters don't have any
// effect in recent Gson versions. See
// https://github.com/google/gson/issues/2787#issuecomment-2581568157
}

/**
Expand Down Expand Up @@ -778,10 +783,6 @@ public GsonBuilder registerTypeHierarchyAdapter(Class<?> baseType, Object typeAd
|| typeAdapter instanceof JsonDeserializer<?>
|| typeAdapter instanceof TypeAdapter<?>);

if (JsonElement.class.isAssignableFrom(baseType)) {
throw new IllegalArgumentException("Cannot override built-in adapter for " + baseType);
}

if (typeAdapter instanceof JsonDeserializer || typeAdapter instanceof JsonSerializer) {
hierarchyFactories.add(TreeTypeAdapter.newTypeHierarchyFactory(baseType, typeAdapter));
}
Expand Down
36 changes: 35 additions & 1 deletion gson/src/test/java/com/google/gson/GsonBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.lang.reflect.Type;
import java.text.DateFormat;
import java.util.Date;
import org.junit.Ignore;
import org.junit.Test;

/**
Expand Down Expand Up @@ -251,7 +252,10 @@ public void testSetStrictness() throws IOException {
public void testRegisterTypeAdapterForObjectAndJsonElements() {
String errorMessage = "Cannot override built-in adapter for ";
Type[] types = {
Object.class, JsonElement.class, JsonArray.class,
Object.class,
// TODO: Registering adapter for JsonElement is allowed (for now) for backward compatibility,
// see https://github.com/google/gson/issues/2787
// JsonElement.class, JsonArray.class,
};
GsonBuilder gsonBuilder = new GsonBuilder();
for (Type type : types) {
Expand All @@ -263,6 +267,22 @@ public void testRegisterTypeAdapterForObjectAndJsonElements() {
}
}

/**
* Verifies that (for now) registering adapter for {@link JsonElement} and subclasses is possible,
* but has no effect. See {@link #testRegisterTypeAdapterForObjectAndJsonElements()}.
*/
@Test
public void testRegisterTypeAdapterForJsonElements() {
Gson gson = new GsonBuilder().registerTypeAdapter(JsonArray.class, NULL_TYPE_ADAPTER).create();
TypeAdapter<JsonArray> adapter = gson.getAdapter(JsonArray.class);
// Does not use registered adapter
assertThat(adapter).isNotSameInstanceAs(NULL_TYPE_ADAPTER);
assertThat(adapter.toJson(new JsonArray())).isEqualTo("[]");
}

@Ignore(
"Registering adapter for JsonElement is allowed (for now) for backward compatibility, see"
+ " https://github.com/google/gson/issues/2787")
@Test
public void testRegisterTypeHierarchyAdapterJsonElements() {
String errorMessage = "Cannot override built-in adapter for ";
Expand All @@ -282,6 +302,20 @@ public void testRegisterTypeHierarchyAdapterJsonElements() {
gsonBuilder.registerTypeHierarchyAdapter(Object.class, NULL_TYPE_ADAPTER);
}

/**
* Verifies that (for now) registering hierarchy adapter for {@link JsonElement} and subclasses is
* possible, but has no effect. See {@link #testRegisterTypeHierarchyAdapterJsonElements()}.
*/
@Test
public void testRegisterTypeHierarchyAdapterJsonElements_Allowed() {
Gson gson =
new GsonBuilder().registerTypeHierarchyAdapter(JsonArray.class, NULL_TYPE_ADAPTER).create();
TypeAdapter<JsonArray> adapter = gson.getAdapter(JsonArray.class);
// Does not use registered adapter
assertThat(adapter).isNotSameInstanceAs(NULL_TYPE_ADAPTER);
assertThat(adapter.toJson(new JsonArray())).isEqualTo("[]");
}

@Test
public void testSetDateFormatWithInvalidPattern() {
GsonBuilder builder = new GsonBuilder();
Expand Down

0 comments on commit a2b1c3c

Please sign in to comment.