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/2563 #2571

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions gson/src/main/java/com/google/gson/Gson.java
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ public Gson() {
this.jsonAdapterFactory = new JsonAdapterAnnotationTypeAdapterFactory(constructorConstructor);
factories.add(jsonAdapterFactory);
factories.add(TypeAdapters.ENUM_FACTORY);
factories.add(TypeAdapters.RAW_ENUM_FACTORY);
factories.add(
new ReflectiveTypeAdapterFactory(
constructorConstructor,
Expand Down
13 changes: 8 additions & 5 deletions gson/src/main/java/com/google/gson/internal/$Gson$Types.java
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,14 @@ public static Class<?> getRawType(Type type) {
Type componentType = ((GenericArrayType) type).getGenericComponentType();
return Array.newInstance(getRawType(componentType), 0).getClass();

} else if (type instanceof TypeVariable) {
// we could use the variable's bounds, but that won't work if there are multiple.
// having a raw type that's more general than necessary is okay
return Object.class;

} else if (type instanceof TypeVariable<?>) {
TypeVariable<?> typeVariable = (TypeVariable<?>) type;
// approximate the raw type with the bound of type type variable, if there are
// multiple bounds, all we can do is picking the first one
Type[] bounds = typeVariable.getBounds();
// Javadoc specifies some bound is always returned, Object if not specified
assert bounds.length > 0;
return getRawType(bounds[0]);
} else if (type instanceof WildcardType) {
Type[] bounds = ((WildcardType) type).getUpperBounds();
// Currently the JLS only permits one bound for wildcards so using first bound is safe
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public final class ObjectTypeAdapter extends TypeAdapter<Object> {
private final Gson gson;
private final ToNumberStrategy toNumberStrategy;

private ObjectTypeAdapter(Gson gson, ToNumberStrategy toNumberStrategy) {
ObjectTypeAdapter(Gson gson, ToNumberStrategy toNumberStrategy) {
this.gson = gson;
this.toNumberStrategy = toNumberStrategy;
}
Expand Down
17 changes: 17 additions & 0 deletions gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.gson.JsonObject;
import com.google.gson.JsonPrimitive;
import com.google.gson.JsonSyntaxException;
import com.google.gson.ToNumberPolicy;
import com.google.gson.TypeAdapter;
import com.google.gson.TypeAdapterFactory;
import com.google.gson.annotations.SerializedName;
Expand Down Expand Up @@ -1030,6 +1031,22 @@ public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
}
};

public static final TypeAdapterFactory RAW_ENUM_FACTORY =
new TypeAdapterFactory() {
@Override
public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
Class<? super T> rawType = typeToken.getRawType();
if (rawType == Enum.class) {
@SuppressWarnings("unchecked")
TypeAdapter<T> adapter =
(TypeAdapter<T>) new ObjectTypeAdapter(gson, ToNumberPolicy.DOUBLE);
return adapter;
} else {
return null;
}
}
};
Comment on lines +1034 to +1048
Copy link
Collaborator

@Marcono1234 Marcono1234 Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this might be functionally equivalent to the previous behavior, I think if we are going to add an adapter for java.lang.Enum anyway, then we should implement it properly and have it behave explicitly as desired (instead of implicitly relying on ObjectTypeAdapter's behavior).

Something like this should work:

public static final TypeAdapterFactory ENUM_BASE_CLASS_FACTORY = new TypeAdapterFactory() {
  @Override
  public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
    if (type.getRawType() != Enum.class) {
      return null;
    }

    return new TypeAdapter<T>() {
      @Override
      public T read(JsonReader in) throws IOException {
        throw new UnsupportedOperationException("Deserializing base class java.lang.Enum is not supported;"
            + " only deserialization of subclasses is supported");
      }

      @Override
      public void write(JsonWriter out, T value) throws IOException {
        if (value == null) {
          out.nullValue();
          return;
        }

        @SuppressWarnings("unchecked")
        TypeAdapter<T> delegate = (TypeAdapter<T>) gson.getAdapter(value.getClass());
        delegate.write(out, value);
      }
    };
  }
};


public static <TT> TypeAdapterFactory newFactory(
final TypeToken<TT> type, final TypeAdapter<TT> typeAdapter) {
return new TypeAdapterFactory() {
Expand Down
96 changes: 96 additions & 0 deletions gson/src/test/java/com/google/gson/HandleRawEnumTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package com.google.gson.functional;

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

import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
import org.junit.Before;
import org.junit.Test;

/**
* Test processing raw enum types.
*
* @author sevcenko
*/
public class HandleRawEnumTest {
private Gson gson;

@Before
public void setUp() throws Exception {
gson = new Gson();
}

public enum SomeEnum {
ONE
}

public static class ClassWithRawEnum {
private final Enum<?> anyEnum;

public ClassWithRawEnum(Enum<?> anyEnum) {
this.anyEnum = anyEnum;
}

public Enum<?> getAnyEnum() {
return anyEnum;
}
}

public static class ClassWithTypedEnum<T extends Enum<T>> {
private final T someEnum;

public ClassWithTypedEnum(T someEnum) {
this.someEnum = someEnum;
}

public T getSomeEnum() {
return someEnum;
}
}

public static class GroupClass {

private final ClassWithTypedEnum<SomeEnum> field;

public GroupClass(ClassWithTypedEnum<SomeEnum> field) {
this.field = field;
}

public ClassWithTypedEnum<SomeEnum> getField() {
return field;
}
}

@Test
public void handleRawEnumClass() {
// serializing raw enum is fine, but note that Adapters.ENUM_FACTORY cannot handle raw enums
// even for serialization! before #2563, this just failed because raw enum falled through
// ReflectiveTypeAdapterFactory, which fails to even search enum for fields
assertThat(gson.toJson(new ClassWithRawEnum(SomeEnum.ONE))).isEqualTo("{\"anyEnum\":\"ONE\"}");

// we can deserialize if the enum type is known
assertThat(
gson.fromJson(
"{\"someEnum\":\"ONE\"}", new TypeToken<ClassWithTypedEnum<SomeEnum>>() {})
.getSomeEnum())
.isEqualTo(SomeEnum.ONE);

assertThat(gson.toJson(new GroupClass(new ClassWithTypedEnum<>(SomeEnum.ONE))))
.isEqualTo("{\"field\":{\"someEnum\":\"ONE\"}}");

assertThat(
gson.fromJson("{\"field\":{\"someEnum\":\"ONE\"}}", GroupClass.class)
.getField()
.getSomeEnum())
.isEqualTo(SomeEnum.ONE);
;
try {
// but raw type cannot be deserialized
gson.fromJson("{\"anyEnum\":\"ONE\"}", new TypeToken<ClassWithRawEnum>() {});
fail("Expected exception");
} catch (IllegalArgumentException e) {
assertThat(e).hasMessageThat().contains("Can not set final java.lang.Enum field");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package com.google.gson.functional;

import static com.google.common.truth.Truth.assertThat;

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.TypeAdapter;
import com.google.gson.TypeAdapterFactory;
import com.google.gson.reflect.TypeToken;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import org.junit.Before;
import org.junit.Test;

/**
* Test deserialization of generic wrapper with type bound.
*
* @author sevcenko
*/
public class InferenceFromTypeVariableTest {
private Gson gson;

@Before
public void setUp() throws Exception {
gson = new GsonBuilder().registerTypeAdapterFactory(new ResolveGenericBoundFactory()).create();
}

public static class Foo {
private final String text;

public Foo(String text) {
this.text = text;
}

public String getText() {
return text;
}
}

public static class BarDynamic<T extends Foo> {
private final T foo;

public BarDynamic(T foo) {
this.foo = foo;
}

public T getFoo() {
return foo;
}
}

static class ResolveGenericBoundFactory implements TypeAdapterFactory {

@SuppressWarnings("unchecked")
@Override
public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
if (type.getType() instanceof TypeVariable<?>) {
TypeVariable<?> tv = (TypeVariable<?>) type.getType();
Type[] bounds = tv.getBounds();
if (bounds.length == 1 && bounds[0] != Object.class) {
Type bound = bounds[0];
return (TypeAdapter<T>) gson.getAdapter(TypeToken.get(bound));
}
}
return null;
}
}

@Test
public void testSubClassSerialization() {
BarDynamic<Foo> bar = new BarDynamic<>(new Foo("foo!"));
assertThat(gson.toJson(bar)).isEqualTo("{\"foo\":{\"text\":\"foo!\"}}");
// without #2563 fix, this would deserialize foo as Object and fails to assign it to foo field
BarDynamic<?> deserialized = gson.fromJson(gson.toJson(bar), BarDynamic.class);
assertThat(deserialized.getFoo().getText()).isEqualTo("foo!");
}
}