Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow registering adapters for JsonElement again #2789

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading