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

issue#2436: Throw exception when registering adapter for Object or JsonElement #2479

Merged
merged 18 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
15a11a9
Code changes and tests for #2436 to throw exception when trying to re…
Aug 21, 2023
0218548
#2436 - Updates to User guide & comments to indicate exception cases …
Aug 25, 2023
0701f89
Merge branch 'main' into feature/issue#2436_enhancement
sachinp97 Aug 25, 2023
f96af9b
#2436 - Fixes as per the review comments.
Aug 28, 2023
840b0b3
Merge branch 'google:main' into feature/issue#2436_enhancement
sachinp97 Aug 28, 2023
4c04356
Merge branch 'feature/issue#2436_enhancement' of https://github.com/s…
Aug 28, 2023
11b97ea
Merge branch 'main' into feature/issue#2436_enhancement
sachinp97 Sep 12, 2023
7fca65d
Merge branch 'main' into feature/issue#2436_enhancement
sachinp97 Sep 12, 2023
d9f42d8
#2436 - Refactored as per latest review comments + throwing error mes…
sachinp97 Sep 16, 2023
02605d2
Merge branch 'feature/issue#2436_enhancement' of https://github.com/s…
sachinp97 Sep 16, 2023
5c0ea74
#2436 - added a clarifying comment in a positive test case.
sachinp97 Sep 16, 2023
263a918
#2436 - formatting and minor changes as per review.
sachinp97 Sep 24, 2023
cc01665
Merge branch 'main' into feature/issue#2436_enhancement
sachinp97 Sep 24, 2023
d853261
Update gson/src/main/java/com/google/gson/GsonBuilder.java
sachinp97 Sep 29, 2023
d9025c8
Update gson/src/test/java/com/google/gson/GsonBuilderTest.java
sachinp97 Sep 29, 2023
ba40b41
Update gson/src/test/java/com/google/gson/GsonBuilderTest.java
sachinp97 Sep 29, 2023
a068738
Merge branch 'main' into feature/issue#2436_enhancement
sachinp97 Sep 29, 2023
fcf2551
Merge branch 'main' into feature/issue#2436_enhancement
sachinp97 Oct 2, 2023
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
4 changes: 3 additions & 1 deletion UserGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,9 @@ gson.registerTypeAdapter(MyType.class, new MyDeserializer());
gson.registerTypeAdapter(MyType.class, new MyInstanceCreator());
```

`registerTypeAdapter` call checks if the type adapter implements more than one of these interfaces and register it for all of them.
`registerTypeAdapter` call checks
1. if the type adapter implements more than one of these interfaces, in that case it registers the adapter for all of them.
2. if the type adapter is for the Object class or JsonElement or any of its subclasses, in that case it throws IllegalArgumentException because overriding the built-in adapters for these types is not supported.

#### Writing a Serializer

Expand Down
19 changes: 19 additions & 0 deletions gson/src/main/java/com/google/gson/GsonBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.google.gson.reflect.TypeToken;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.text.DateFormat;
import java.util.ArrayDeque;
Expand Down Expand Up @@ -664,6 +665,7 @@ public GsonBuilder setDateFormat(int dateStyle, int timeStyle) {
* @param typeAdapter This object must implement at least one of the {@link TypeAdapter},
* {@link InstanceCreator}, {@link JsonSerializer}, and a {@link JsonDeserializer} interfaces.
* @return a reference to this {@code GsonBuilder} object to fulfill the "Builder" pattern
* @throws IllegalArgumentException if the type adapter being registered is for {@code Object} class or {@link JsonElement} or any of its subclasses
*/
@CanIgnoreReturnValue
public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
Expand All @@ -672,6 +674,11 @@ public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
|| typeAdapter instanceof JsonDeserializer<?>
|| typeAdapter instanceof InstanceCreator<?>
|| typeAdapter instanceof TypeAdapter<?>);

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

if (typeAdapter instanceof InstanceCreator<?>) {
instanceCreators.put(type, (InstanceCreator<?>) typeAdapter);
}
Expand All @@ -687,6 +694,12 @@ public GsonBuilder registerTypeAdapter(Type type, Object typeAdapter) {
return this;
}

private boolean isTypeObjectOrJsonElement(Type type) {
return type instanceof Class &&
(type == Object.class
|| JsonElement.class.isAssignableFrom((Class<?>) type));
}

/**
* Register a factory for type adapters. Registering a factory is useful when the type
* adapter needs to be configured based on the type of the field being processed. Gson
Expand Down Expand Up @@ -718,6 +731,7 @@ public GsonBuilder registerTypeAdapterFactory(TypeAdapterFactory factory) {
* @param typeAdapter This object must implement at least one of {@link TypeAdapter},
* {@link JsonSerializer} or {@link JsonDeserializer} interfaces.
* @return a reference to this {@code GsonBuilder} object to fulfill the "Builder" pattern
* @throws IllegalArgumentException if the type adapter being registered is for {@link JsonElement} or any of its subclasses
* @since 1.7
*/
@CanIgnoreReturnValue
Expand All @@ -726,6 +740,11 @@ public GsonBuilder registerTypeHierarchyAdapter(Class<?> baseType, Object typeAd
$Gson$Preconditions.checkArgument(typeAdapter instanceof JsonSerializer<?>
|| 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: 36 additions & 0 deletions gson/src/test/java/com/google/gson/GsonBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.google.gson;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;

import com.google.gson.stream.JsonReader;
Expand Down Expand Up @@ -254,4 +255,39 @@ public void testSetStrictness() throws IOException {
assertThat(gson.newJsonReader(new StringReader("{}")).getStrictness()).isEqualTo(STRICTNESS);
assertThat(gson.newJsonWriter(new StringWriter()).getStrictness()).isEqualTo(STRICTNESS);
}

@Test
public void testRegisterTypeAdapterForObjectAndJsonElements() {
final String ERROR_MESSAGE = "Cannot override built-in adapter for ";
Type[] types = {
Object.class,
JsonElement.class,
JsonArray.class,
};
GsonBuilder gsonBuilder = new GsonBuilder();
for (Type type : types) {
IllegalArgumentException e = assertThrows(IllegalArgumentException.class,
() -> gsonBuilder.registerTypeAdapter(type, NULL_TYPE_ADAPTER));
assertThat(e).hasMessageThat().isEqualTo(ERROR_MESSAGE + type);
}
}


@Test
public void testRegisterTypeHierarchyAdapterJsonElements() {
final String ERROR_MESSAGE = "Cannot override built-in adapter for ";
Class<?>[] types = {
JsonElement.class,
JsonArray.class,
};
GsonBuilder gsonBuilder = new GsonBuilder();
for (Class<?> type : types) {
IllegalArgumentException e = assertThrows(IllegalArgumentException.class,
() -> gsonBuilder.registerTypeHierarchyAdapter(type, NULL_TYPE_ADAPTER));

assertThat(e).hasMessageThat().isEqualTo(ERROR_MESSAGE + type);
}
// But registering type hierarchy adapter for Object should be allowed
gsonBuilder.registerTypeHierarchyAdapter(Object.class, NULL_TYPE_ADAPTER);
sachinp97 marked this conversation as resolved.
Show resolved Hide resolved
}
}
2 changes: 1 addition & 1 deletion gson/src/test/java/com/google/gson/GsonTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void testClonedTypeAdapterFactoryListsAreIndependent() {
Collections.<ReflectionAccessFilter>emptyList());

Gson clone = original.newBuilder()
.registerTypeAdapter(Object.class, new TestTypeAdapter())
.registerTypeAdapter(int.class, new TestTypeAdapter())
.create();

assertThat(clone.factories).hasSize(original.factories.size() + 1);
Expand Down