Skip to content

Commit

Permalink
Fix TypeAdapterRuntimeTypeWrapper not detecting reflective TreeTypeAd…
Browse files Browse the repository at this point in the history
…apter and FutureTypeAdapter (#1787)

* Fix TypeAdapterRuntimeTypeWrapper not detecting reflective TreeTypeAdapter

Previously on serialization TypeAdapterRuntimeTypeWrapper preferred a
TreeTypeAdapter without `serializer` which falls back to the reflective
adapter. This behavior was incorrect because it caused the reflective
adapter for a Base class to be used for serialization (indirectly as
TreeTypeAdapter delegate) instead of using the reflective adapter for
a Subclass extending Base.

* Address review feedback

* Convert TypeAdapterRuntimeTypeWrapperTest to JUnit 4 test

* Prefer wrapped reflective adapter for serialization of subclass

* Detect reflective adapter used as delegate for Gson.FutureTypeAdapter

* Tiny style tweak.

Co-authored-by: Éamonn McManus <emcmanus@google.com>
  • Loading branch information
Marcono1234 and eamonnmcmanus committed Oct 10, 2022
1 parent 5269701 commit 8451c1f
Show file tree
Hide file tree
Showing 5 changed files with 256 additions and 14 deletions.
22 changes: 14 additions & 8 deletions gson/src/main/java/com/google/gson/Gson.java
Expand Up @@ -32,6 +32,7 @@
import com.google.gson.internal.bind.NumberTypeAdapter;
import com.google.gson.internal.bind.ObjectTypeAdapter;
import com.google.gson.internal.bind.ReflectiveTypeAdapterFactory;
import com.google.gson.internal.bind.SerializationDelegatingTypeAdapter;
import com.google.gson.internal.bind.TypeAdapters;
import com.google.gson.internal.sql.SqlTypesSupport;
import com.google.gson.reflect.TypeToken;
Expand Down Expand Up @@ -1315,7 +1316,7 @@ public <T> T fromJson(JsonElement json, TypeToken<T> typeOfT) throws JsonSyntaxE
return fromJson(new JsonTreeReader(json), typeOfT);
}

static class FutureTypeAdapter<T> extends TypeAdapter<T> {
static class FutureTypeAdapter<T> extends SerializationDelegatingTypeAdapter<T> {
private TypeAdapter<T> delegate;

public void setDelegate(TypeAdapter<T> typeAdapter) {
Expand All @@ -1325,18 +1326,23 @@ public void setDelegate(TypeAdapter<T> typeAdapter) {
delegate = typeAdapter;
}

@Override public T read(JsonReader in) throws IOException {
private TypeAdapter<T> delegate() {
if (delegate == null) {
throw new IllegalStateException();
throw new IllegalStateException("Delegate has not been set yet");
}
return delegate.read(in);
return delegate;
}

@Override public TypeAdapter<T> getSerializationDelegate() {
return delegate();
}

@Override public T read(JsonReader in) throws IOException {
return delegate().read(in);
}

@Override public void write(JsonWriter out, T value) throws IOException {
if (delegate == null) {
throw new IllegalStateException();
}
delegate.write(out, value);
delegate().write(out, value);
}
}

Expand Down
@@ -0,0 +1,14 @@
package com.google.gson.internal.bind;

import com.google.gson.TypeAdapter;

/**
* Type adapter which might delegate serialization to another adapter.
*/
public abstract class SerializationDelegatingTypeAdapter<T> extends TypeAdapter<T> {
/**
* Returns the adapter used for serialization, might be {@code this} or another adapter.
* That other adapter might itself also be a {@code SerializationDelegatingTypeAdapter}.
*/
public abstract TypeAdapter<T> getSerializationDelegate();
}
Expand Up @@ -38,7 +38,7 @@
* tree adapter may be serialization-only or deserialization-only, this class
* has a facility to lookup a delegate type adapter on demand.
*/
public final class TreeTypeAdapter<T> extends TypeAdapter<T> {
public final class TreeTypeAdapter<T> extends SerializationDelegatingTypeAdapter<T> {
private final JsonSerializer<T> serializer;
private final JsonDeserializer<T> deserializer;
final Gson gson;
Expand Down Expand Up @@ -97,6 +97,15 @@ private TypeAdapter<T> delegate() {
: (delegate = gson.getDelegateAdapter(skipPast, typeToken));
}

/**
* Returns the type adapter which is used for serialization. Returns {@code this}
* if this {@code TreeTypeAdapter} has a {@link #serializer}; otherwise returns
* the delegate.
*/
@Override public TypeAdapter<T> getSerializationDelegate() {
return serializer != null ? this : delegate();
}

/**
* Returns a new factory that will match each type against {@code exactType}.
*/
Expand Down Expand Up @@ -169,5 +178,5 @@ private final class GsonContextImpl implements JsonSerializationContext, JsonDes
@Override public <R> R deserialize(JsonElement json, Type typeOfT) throws JsonParseException {
return (R) gson.fromJson(json, typeOfT);
}
};
}
}
Expand Up @@ -53,10 +53,12 @@ public void write(JsonWriter out, T value) throws IOException {
if (runtimeType != type) {
@SuppressWarnings("unchecked")
TypeAdapter<T> runtimeTypeAdapter = (TypeAdapter<T>) context.getAdapter(TypeToken.get(runtimeType));
// For backward compatibility only check ReflectiveTypeAdapterFactory.Adapter here but not any other
// wrapping adapters, see https://github.com/google/gson/pull/1787#issuecomment-1222175189
if (!(runtimeTypeAdapter instanceof ReflectiveTypeAdapterFactory.Adapter)) {
// The user registered a type adapter for the runtime type, so we will use that
chosen = runtimeTypeAdapter;
} else if (!(delegate instanceof ReflectiveTypeAdapterFactory.Adapter)) {
} else if (!isReflective(delegate)) {
// The user registered a type adapter for Base class, so we prefer it over the
// reflective type adapter for the runtime type
chosen = delegate;
Expand All @@ -68,12 +70,30 @@ public void write(JsonWriter out, T value) throws IOException {
chosen.write(out, value);
}

/**
* Returns whether the type adapter uses reflection.
*
* @param typeAdapter the type adapter to check.
*/
private static boolean isReflective(TypeAdapter<?> typeAdapter) {
// Run this in loop in case multiple delegating adapters are nested
while (typeAdapter instanceof SerializationDelegatingTypeAdapter) {
TypeAdapter<?> delegate = ((SerializationDelegatingTypeAdapter<?>) typeAdapter).getSerializationDelegate();
// Break if adapter does not delegate serialization
if (delegate == typeAdapter) {
break;
}
typeAdapter = delegate;
}

return typeAdapter instanceof ReflectiveTypeAdapterFactory.Adapter;
}

/**
* Finds a compatible runtime type if it is more specific
*/
private Type getRuntimeTypeIfMoreSpecific(Type type, Object value) {
if (value != null
&& (type == Object.class || type instanceof TypeVariable<?> || type instanceof Class<?>)) {
private static Type getRuntimeTypeIfMoreSpecific(Type type, Object value) {
if (value != null && (type instanceof Class<?> || type instanceof TypeVariable<?>)) {
type = value.getClass();
}
return type;
Expand Down
@@ -0,0 +1,193 @@
package com.google.gson.functional;

import static org.junit.Assert.assertEquals;

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonDeserializationContext;
import com.google.gson.JsonDeserializer;
import com.google.gson.JsonElement;
import com.google.gson.JsonPrimitive;
import com.google.gson.JsonSerializationContext;
import com.google.gson.JsonSerializer;
import com.google.gson.TypeAdapter;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
import java.io.IOException;
import java.lang.reflect.Type;
import org.junit.Test;

public class TypeAdapterRuntimeTypeWrapperTest {
private static class Base {
}
private static class Subclass extends Base {
@SuppressWarnings("unused")
String f = "test";
}
private static class Container {
@SuppressWarnings("unused")
Base b = new Subclass();
}
private static class Deserializer implements JsonDeserializer<Base> {
@Override
public Base deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) {
throw new AssertionError("not needed for this test");
}
}

/**
* When custom {@link JsonSerializer} is registered for Base should
* prefer that over reflective adapter for Subclass for serialization.
*/
@Test
public void testJsonSerializer() {
Gson gson = new GsonBuilder()
.registerTypeAdapter(Base.class, new JsonSerializer<Base>() {
@Override
public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) {
return new JsonPrimitive("serializer");
}
})
.create();

String json = gson.toJson(new Container());
assertEquals("{\"b\":\"serializer\"}", json);
}

/**
* When only {@link JsonDeserializer} is registered for Base, then on
* serialization should prefer reflective adapter for Subclass since
* Base would use reflective adapter as delegate.
*/
@Test
public void testJsonDeserializer_ReflectiveSerializerDelegate() {
Gson gson = new GsonBuilder()
.registerTypeAdapter(Base.class, new Deserializer())
.create();

String json = gson.toJson(new Container());
assertEquals("{\"b\":{\"f\":\"test\"}}", json);
}

/**
* When {@link JsonDeserializer} with custom adapter as delegate is
* registered for Base, then on serialization should prefer custom adapter
* delegate for Base over reflective adapter for Subclass.
*/
@Test
public void testJsonDeserializer_CustomSerializerDelegate() {
Gson gson = new GsonBuilder()
// Register custom delegate
.registerTypeAdapter(Base.class, new TypeAdapter<Base>() {
@Override
public Base read(JsonReader in) throws IOException {
throw new UnsupportedOperationException();
}
@Override
public void write(JsonWriter out, Base value) throws IOException {
out.value("custom delegate");
}
})
.registerTypeAdapter(Base.class, new Deserializer())
.create();

String json = gson.toJson(new Container());
assertEquals("{\"b\":\"custom delegate\"}", json);
}

/**
* When two (or more) {@link JsonDeserializer}s are registered for Base
* which eventually fall back to reflective adapter as delegate, then on
* serialization should prefer reflective adapter for Subclass.
*/
@Test
public void testJsonDeserializer_ReflectiveTreeSerializerDelegate() {
Gson gson = new GsonBuilder()
// Register delegate which itself falls back to reflective serialization
.registerTypeAdapter(Base.class, new Deserializer())
.registerTypeAdapter(Base.class, new Deserializer())
.create();

String json = gson.toJson(new Container());
assertEquals("{\"b\":{\"f\":\"test\"}}", json);
}

/**
* When {@link JsonDeserializer} with {@link JsonSerializer} as delegate
* is registered for Base, then on serialization should prefer
* {@code JsonSerializer} over reflective adapter for Subclass.
*/
@Test
public void testJsonDeserializer_JsonSerializerDelegate() {
Gson gson = new GsonBuilder()
// Register JsonSerializer as delegate
.registerTypeAdapter(Base.class, new JsonSerializer<Base>() {
@Override
public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) {
return new JsonPrimitive("custom delegate");
}
})
.registerTypeAdapter(Base.class, new Deserializer())
.create();

String json = gson.toJson(new Container());
assertEquals("{\"b\":\"custom delegate\"}", json);
}

/**
* When a {@link JsonDeserializer} is registered for Subclass, and a custom
* {@link JsonSerializer} is registered for Base, then Gson should prefer
* the reflective adapter for Subclass for backward compatibility (see
* https://github.com/google/gson/pull/1787#issuecomment-1222175189) even
* though normally TypeAdapterRuntimeTypeWrapper should prefer the custom
* serializer for Base.
*/
@Test
public void testJsonDeserializer_SubclassBackwardCompatibility() {
Gson gson = new GsonBuilder()
.registerTypeAdapter(Subclass.class, new JsonDeserializer<Subclass>() {
@Override
public Subclass deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) {
throw new AssertionError("not needed for this test");
}
})
.registerTypeAdapter(Base.class, new JsonSerializer<Base>() {
@Override
public JsonElement serialize(Base src, Type typeOfSrc, JsonSerializationContext context) {
return new JsonPrimitive("base");
}
})
.create();

String json = gson.toJson(new Container());
assertEquals("{\"b\":{\"f\":\"test\"}}", json);
}

private static class CyclicBase {
@SuppressWarnings("unused")
CyclicBase f;
}

private static class CyclicSub extends CyclicBase {
@SuppressWarnings("unused")
int i;

public CyclicSub(int i) {
this.i = i;
}
}

/**
* Tests behavior when the type of a field refers to a type whose adapter is
* currently in the process of being created. For these cases {@link Gson}
* uses a future adapter for the type. That adapter later uses the actual
* adapter as delegate.
*/
@Test
public void testGsonFutureAdapter() {
CyclicBase b = new CyclicBase();
b.f = new CyclicSub(2);
String json = new Gson().toJson(b);
assertEquals("{\"f\":{\"i\":2}}", json);
}
}

0 comments on commit 8451c1f

Please sign in to comment.