Adjust Record adapter and extend test coverage (#2224)

* Adjust Record adapter and extend test coverage

* Address review feedback

* Make constructor string more concise

* Add tests for Gson default behavior for static fields

* Improve exception for deserializing static final field

Previously it would report "Unexpected IllegalAccessException occurred..."
due to the uncaught IllegalAccessException.

* Improve handling of exception thrown by accessor

Such an exception is not 'unexpected' (which was claimed by the previous
exception handling) because user code could throw it.

* Improve constructor invocation exception handling and add tests
This commit is contained in:
Marcono1234 2022-10-22 18:01:56 +02:00 committed by GitHub
parent 954d526af4
commit 66d9621ce8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 715 additions and 211 deletions

View File

@ -261,14 +261,17 @@ public final class ConstructorConstructor {
@SuppressWarnings("unchecked") // T is the same raw type as is requested
T newInstance = (T) constructor.newInstance();
return newInstance;
} catch (InstantiationException e) {
// TODO: JsonParseException ?
throw new RuntimeException("Failed to invoke " + constructor + " with no args", e);
}
// Note: InstantiationException should be impossible because check at start of method made sure
// that class is not abstract
catch (InstantiationException e) {
throw new RuntimeException("Failed to invoke constructor '" + ReflectionHelper.constructorToString(constructor) + "'"
+ " with no args", e);
} catch (InvocationTargetException e) {
// TODO: don't wrap if cause is unchecked!
// TODO: don't wrap if cause is unchecked?
// TODO: JsonParseException ?
throw new RuntimeException("Failed to invoke " + constructor + " with no args",
e.getTargetException());
throw new RuntimeException("Failed to invoke constructor '" + ReflectionHelper.constructorToString(constructor) + "'"
+ " with no args", e.getCause());
} catch (IllegalAccessException e) {
throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e);
}

View File

@ -19,6 +19,7 @@ package com.google.gson.internal.bind;
import com.google.gson.FieldNamingStrategy;
import com.google.gson.Gson;
import com.google.gson.JsonIOException;
import com.google.gson.JsonParseException;
import com.google.gson.JsonSyntaxException;
import com.google.gson.ReflectionAccessFilter;
import com.google.gson.ReflectionAccessFilter.FilterResult;
@ -38,16 +39,19 @@ import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonToken;
import com.google.gson.stream.JsonWriter;
import java.io.IOException;
import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@ -108,8 +112,7 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
ReflectionAccessFilterHelper.getFilterResult(reflectionFilters, raw);
if (filterResult == FilterResult.BLOCK_ALL) {
throw new JsonIOException(
"ReflectionAccessFilter does not permit using reflection for "
+ raw
"ReflectionAccessFilter does not permit using reflection for " + raw
+ ". Register a TypeAdapter for this type or adjust the access filter.");
}
boolean blockInaccessible = filterResult == FilterResult.BLOCK_INACCESSIBLE;
@ -117,19 +120,22 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
// If the type is actually a Java Record, we need to use the RecordAdapter instead. This will always be false
// on JVMs that do not support records.
if (ReflectionHelper.isRecord(raw)) {
return new RecordAdapter<>(raw, getBoundFields(gson, type, raw, true, true));
@SuppressWarnings("unchecked")
TypeAdapter<T> adapter = (TypeAdapter<T>) new RecordAdapter<>(raw,
getBoundFields(gson, type, raw, blockInaccessible, true), blockInaccessible);
return adapter;
}
ObjectConstructor<T> constructor = constructorConstructor.get(type);
return new FieldReflectionAdapter<>(constructor, getBoundFields(gson, type, raw, blockInaccessible, false));
}
private static void checkAccessible(Object object, Field field) {
if (!ReflectionAccessFilterHelper.canAccess(field, Modifier.isStatic(field.getModifiers()) ? null : object)) {
throw new JsonIOException("Field '" + field.getDeclaringClass().getName() + "#"
+ field.getName() + "' is not accessible and ReflectionAccessFilter does not "
+ "permit making it accessible. Register a TypeAdapter for the declaring type "
+ "or adjust the access filter.");
private static <M extends AccessibleObject & Member> void checkAccessible(Object object, M member) {
if (!ReflectionAccessFilterHelper.canAccess(member, Modifier.isStatic(member.getModifiers()) ? null : object)) {
String memberDescription = ReflectionHelper.getAccessibleObjectDescription(member, true);
throw new JsonIOException(memberDescription + " is not accessible and ReflectionAccessFilter does not"
+ " permit making it accessible. Register a TypeAdapter for the declaring type, adjust the"
+ " access filter or increase the visibility of the element and its declaring type.");
}
}
@ -137,7 +143,12 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
final Gson context, final Field field, final Method accessor, final String name,
final TypeToken<?> fieldType, boolean serialize, boolean deserialize,
final boolean blockInaccessible) {
final boolean isPrimitive = Primitives.isPrimitive(fieldType.getRawType());
int modifiers = field.getModifiers();
final boolean isStaticFinalField = Modifier.isStatic(modifiers) && Modifier.isFinal(modifiers);
JsonAdapter annotation = field.getAnnotation(JsonAdapter.class);
TypeAdapter<?> mapped = null;
if (annotation != null) {
@ -152,15 +163,29 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
final TypeAdapter<Object> typeAdapter = (TypeAdapter<Object>) mapped;
return new ReflectiveTypeAdapterFactory.BoundField(name, field.getName(), serialize, deserialize) {
@Override void write(JsonWriter writer, Object source)
throws IOException, ReflectiveOperationException {
throws IOException, IllegalAccessException {
if (!serialized) return;
if (blockInaccessible && accessor == null) {
if (blockInaccessible) {
if (accessor == null) {
checkAccessible(source, field);
} else {
// Note: This check might actually be redundant because access check for canonical
// constructor should have failed already
checkAccessible(source, accessor);
}
}
Object fieldValue = (accessor != null)
? accessor.invoke(source)
: field.get(source);
Object fieldValue;
if (accessor != null) {
try {
fieldValue = accessor.invoke(source);
} catch (InvocationTargetException e) {
String accessorDescription = ReflectionHelper.getAccessibleObjectDescription(accessor, false);
throw new JsonIOException("Accessor " + accessorDescription + " threw exception", e.getCause());
}
} else {
fieldValue = field.get(source);
}
if (fieldValue == source) {
// avoid direct recursion
return;
@ -172,11 +197,13 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
}
@Override
void readIntoArray(JsonReader reader, int index, Object[] target) throws IOException {
void readIntoArray(JsonReader reader, int index, Object[] target) throws IOException, JsonParseException {
Object fieldValue = typeAdapter.read(reader);
if (fieldValue != null || !isPrimitive) {
target[index] = fieldValue;
if (fieldValue == null && isPrimitive) {
throw new JsonParseException("null is not allowed as value for record component '" + fieldName + "'"
+ " of primitive type; at path " + reader.getPath());
}
target[index] = fieldValue;
}
@Override
@ -186,6 +213,11 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
if (fieldValue != null || !isPrimitive) {
if (blockInaccessible) {
checkAccessible(target, field);
} else if (isStaticFinalField) {
// Reflection does not permit setting value of `static final` field, even after calling `setAccessible`
// Handle this here to avoid causing IllegalAccessException when calling `Field.set`
String fieldDescription = ReflectionHelper.getAccessibleObjectDescription(field, false);
throw new JsonIOException("Cannot set value of 'static final' " + fieldDescription);
}
field.set(target, fieldValue);
}
@ -209,8 +241,8 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
if (raw != originalRaw && fields.length > 0) {
FilterResult filterResult = ReflectionAccessFilterHelper.getFilterResult(reflectionFilters, raw);
if (filterResult == FilterResult.BLOCK_ALL) {
throw new JsonIOException("ReflectionAccessFilter does not permit using reflection for "
+ raw + " (supertype of " + originalRaw + "). Register a TypeAdapter for this type "
throw new JsonIOException("ReflectionAccessFilter does not permit using reflection for " + raw
+ " (supertype of " + originalRaw + "). Register a TypeAdapter for this type"
+ " or adjust the access filter.");
}
blockInaccessible = filterResult == FilterResult.BLOCK_INACCESSIBLE;
@ -224,18 +256,34 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
}
// The accessor method is only used for records. If the type is a record, we will read out values
// via its accessor method instead of via reflection. This way we will bypass the accessible restrictions
// If there is a static field on a record, there will not be an accessor. Instead we will use the default
// field logic for dealing with statics.
Method accessor = null;
if (isRecord && !Modifier.isStatic(field.getModifiers())) {
if (isRecord) {
// If there is a static field on a record, there will not be an accessor. Instead we will use the default
// field serialization logic, but for deserialization the field is excluded for simplicity. Note that Gson
// ignores static fields by default, but GsonBuilder.excludeFieldsWithModifiers can overwrite this.
if (Modifier.isStatic(field.getModifiers())) {
deserialize = false;
} else {
accessor = ReflectionHelper.getAccessor(raw, field);
// If blockInaccessible, skip and perform access check later
if (!blockInaccessible) {
ReflectionHelper.makeAccessible(accessor);
}
// If blockInaccessible, skip and perform access check later. When constructing a BoundedField for a Record
// field, blockInaccessible is always true, thus makeAccessible will never get called. This is not an issue
// though, as we will use the accessor method instead for reading record fields, and the constructor for
// writing fields.
if (!blockInaccessible) {
// @SerializedName can be placed on accessor method, but it is not supported there
// If field and method have annotation it is not easily possible to determine if accessor method
// is implicit and has inherited annotation, or if it is explicitly declared with custom annotation
if (accessor.getAnnotation(SerializedName.class) != null
&& field.getAnnotation(SerializedName.class) == null) {
String methodDescription = ReflectionHelper.getAccessibleObjectDescription(accessor, false);
throw new JsonIOException("@SerializedName on " + methodDescription + " is not supported");
}
}
}
// If blockInaccessible, skip and perform access check later
// For Records if the accessor method is used the field does not have to be made accessible
if (!blockInaccessible && accessor == null) {
ReflectionHelper.makeAccessible(field);
}
Type fieldType = $Gson$Types.resolve(type.getType(), raw, field.getGenericType());
@ -275,10 +323,10 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
}
/** Read this field value from the source, and append its JSON value to the writer */
abstract void write(JsonWriter writer, Object source) throws IOException, ReflectiveOperationException;
abstract void write(JsonWriter writer, Object source) throws IOException, IllegalAccessException;
/** Read the value into the target array, used to provide constructor arguments for records */
abstract void readIntoArray(JsonReader reader, int index, Object[] target) throws IOException;
abstract void readIntoArray(JsonReader reader, int index, Object[] target) throws IOException, JsonParseException;
/** Read the value from the reader, and set it on the corresponding field on target via reflection */
abstract void readIntoField(JsonReader reader, Object target) throws IOException, IllegalAccessException;
@ -297,10 +345,11 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
* @param <T> type of objects that this Adapter creates.
* @param <A> type of accumulator used to build the deserialization result.
*/
// This class is public because external projects check for this class with `instanceof` (even though it is internal)
public static abstract class Adapter<T, A> extends TypeAdapter<T> {
protected final Map<String, BoundField> boundFields;
final Map<String, BoundField> boundFields;
protected Adapter(Map<String, BoundField> boundFields) {
Adapter(Map<String, BoundField> boundFields) {
this.boundFields = boundFields;
}
@ -318,8 +367,6 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
}
} catch (IllegalAccessException e) {
throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e);
} catch (ReflectiveOperationException e) {
throw ReflectionHelper.createExceptionForRecordReflectionException(e);
}
out.endObject();
}
@ -356,7 +403,7 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
/** Create the Object that will be used to collect each field value */
abstract A createAccumulator();
/**
* Read a single BoundedField into the accumulator. The JsonReader will be pointed at the
* Read a single BoundField into the accumulator. The JsonReader will be pointed at the
* start of the value for the BoundField to read from.
*/
abstract void readField(A accumulator, JsonReader in, BoundField field)
@ -391,20 +438,25 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
}
private static final class RecordAdapter<T> extends Adapter<T, Object[]> {
static Map<Class<?>, Object> PRIMITIVE_DEFAULTS = primitiveDefaults();
static final Map<Class<?>, Object> PRIMITIVE_DEFAULTS = primitiveDefaults();
// The actual record constructor.
private final Constructor<? super T> constructor;
// The canonical constructor of the record
private final Constructor<T> constructor;
// Array of arguments to the constructor, initialized with default values for primitives
private final Object[] constructorArgsDefaults;
// Map from component names to index into the constructors arguments.
private final Map<String, Integer> componentIndices = new HashMap<>();
RecordAdapter(Class<? super T> raw, Map<String, BoundField> boundFields) {
RecordAdapter(Class<T> raw, Map<String, BoundField> boundFields, boolean blockInaccessible) {
super(boundFields);
this.constructor = ReflectionHelper.getCanonicalRecordConstructor(raw);
constructor = ReflectionHelper.getCanonicalRecordConstructor(raw);
if (blockInaccessible) {
checkAccessible(null, constructor);
} else {
// Ensure the constructor is accessible
ReflectionHelper.makeAccessible(this.constructor);
ReflectionHelper.makeAccessible(constructor);
}
String[] componentNames = ReflectionHelper.getRecordComponentNames(raw);
for (int i = 0; i < componentNames.length; i++) {
@ -441,29 +493,39 @@ public final class ReflectiveTypeAdapterFactory implements TypeAdapterFactory {
@Override
void readField(Object[] accumulator, JsonReader in, BoundField field) throws IOException {
Integer fieldIndex = componentIndices.get(field.fieldName);
if (fieldIndex == null) {
// Obtain the component index from the name of the field backing it
Integer componentIndex = componentIndices.get(field.fieldName);
if (componentIndex == null) {
throw new IllegalStateException(
"Could not find the index in the constructor "
+ constructor
+ " for field with name "
+ field.name
+ ", unable to determine which argument in the constructor the field corresponds"
+ " to. This is unexpected behaviour, as we expect the RecordComponents to have the"
"Could not find the index in the constructor '" + ReflectionHelper.constructorToString(constructor) + "'"
+ " for field with name '" + field.fieldName + "',"
+ " unable to determine which argument in the constructor the field corresponds"
+ " to. This is unexpected behavior, as we expect the RecordComponents to have the"
+ " same names as the fields in the Java class, and that the order of the"
+ " RecordComponents is the same as the order of the canonical arguments.");
+ " RecordComponents is the same as the order of the canonical constructor parameters.");
}
field.readIntoArray(in, fieldIndex, accumulator);
field.readIntoArray(in, componentIndex, accumulator);
}
@Override
@SuppressWarnings("unchecked")
T finalize(Object[] accumulator) {
try {
return (T) constructor.newInstance(accumulator);
} catch (ReflectiveOperationException e) {
return constructor.newInstance(accumulator);
} catch (IllegalAccessException e) {
throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e);
}
// Note: InstantiationException should be impossible because record class is not abstract;
// IllegalArgumentException should not be possible unless a bad adapter returns objects of the wrong type
catch (InstantiationException | IllegalArgumentException e) {
throw new RuntimeException(
"Failed to invoke " + constructor + " with args " + Arrays.toString(accumulator), e);
"Failed to invoke constructor '" + ReflectionHelper.constructorToString(constructor) + "'"
+ " with args " + Arrays.toString(accumulator), e);
}
catch (InvocationTargetException e) {
// TODO: JsonParseException ?
throw new RuntimeException(
"Failed to invoke constructor '" + ReflectionHelper.constructorToString(constructor) + "'"
+ " with args " + Arrays.toString(accumulator), e.getCause());
}
}
}

View File

@ -24,57 +24,75 @@ public class ReflectionHelper {
private ReflectionHelper() {}
/**
* Tries making the field accessible, wrapping any thrown exception in a {@link JsonIOException}
* with descriptive message.
*
* @param field field to make accessible
* @throws JsonIOException if making the field accessible fails
*/
public static void makeAccessible(Field field) throws JsonIOException {
makeAccessible("field '" + field.getDeclaringClass().getName() + "#" + field.getName() + "'", field);
}
/**
* Tries making the constructor accessible, wrapping any thrown exception in a {@link JsonIOException}
* with descriptive message.
*
* @param constructor constructor to make accessible
* @throws JsonIOException if making the constructor accessible fails
*/
public static void makeAccessible(Constructor<?> constructor) throws JsonIOException {
makeAccessible(
"constructor " + constructor + " in " + constructor.getDeclaringClass().getName(),
constructor
);
}
/**
* Internal implementation of making an {@link AccessibleObject} accessible.
*
* @param description describe what we are attempting to make accessible
* @param object the object that {@link AccessibleObject#setAccessible(boolean)} should be called on.
* @throws JsonIOException if making the object accessible fails
*/
private static void makeAccessible(String description, AccessibleObject object) throws JsonIOException {
public static void makeAccessible(AccessibleObject object) throws JsonIOException {
try {
object.setAccessible(true);
} catch (Exception exception) {
throw new JsonIOException("Failed making " + description + "' accessible; either change its visibility "
+ "or write a custom TypeAdapter for its declaring type", exception);
String description = getAccessibleObjectDescription(object, false);
throw new JsonIOException("Failed making " + description + " accessible; either increase its visibility"
+ " or write a custom TypeAdapter for its declaring type.", exception);
}
}
/**
* Returns a short string describing the {@link AccessibleObject} in a human-readable way.
* The result is normally shorter than {@link AccessibleObject#toString()} because it omits
* modifiers (e.g. {@code final}) and uses simple names for constructor and method parameter
* types.
*
* @param object object to describe
* @param uppercaseFirstLetter whether the first letter of the description should be uppercased
*/
public static String getAccessibleObjectDescription(AccessibleObject object, boolean uppercaseFirstLetter) {
String description;
if (object instanceof Field) {
Field field = (Field) object;
description = "field '" + field.getDeclaringClass().getName() + "#" + field.getName() + "'";
} else if (object instanceof Method) {
Method method = (Method) object;
StringBuilder methodSignatureBuilder = new StringBuilder(method.getName());
appendExecutableParameters(method, methodSignatureBuilder);
String methodSignature = methodSignatureBuilder.toString();
description = "method '" + method.getDeclaringClass().getName() + "#" + methodSignature + "'";
} else if (object instanceof Constructor) {
description = "constructor '" + constructorToString((Constructor<?>) object) + "'";
} else {
description = "<unknown AccessibleObject> " + object.toString();
}
if (uppercaseFirstLetter && Character.isLowerCase(description.charAt(0))) {
description = Character.toUpperCase(description.charAt(0)) + description.substring(1);
}
return description;
}
/**
* Creates a string representation for a constructor.
* E.g.: {@code java.lang.String#String(char[], int, int)}
* E.g.: {@code java.lang.String(char[], int, int)}
*/
private static String constructorToString(Constructor<?> constructor) {
StringBuilder stringBuilder = new StringBuilder(constructor.getDeclaringClass().getName())
.append('#')
.append(constructor.getDeclaringClass().getSimpleName())
.append('(');
Class<?>[] parameters = constructor.getParameterTypes();
public static String constructorToString(Constructor<?> constructor) {
StringBuilder stringBuilder = new StringBuilder(constructor.getDeclaringClass().getName());
appendExecutableParameters(constructor, stringBuilder);
return stringBuilder.toString();
}
// Note: Ideally parameter type would be java.lang.reflect.Executable, but that was added in Java 8
private static void appendExecutableParameters(AccessibleObject executable, StringBuilder stringBuilder) {
stringBuilder.append('(');
Class<?>[] parameters = (executable instanceof Method)
? ((Method) executable).getParameterTypes()
: ((Constructor<?>) executable).getParameterTypes();
for (int i = 0; i < parameters.length; i++) {
if (i > 0) {
stringBuilder.append(", ");
@ -82,7 +100,7 @@ public class ReflectionHelper {
stringBuilder.append(parameters[i].getSimpleName());
}
return stringBuilder.append(')').toString();
stringBuilder.append(')');
}
/**
@ -99,9 +117,9 @@ public class ReflectionHelper {
return null;
} catch (Exception exception) {
return "Failed making constructor '" + constructorToString(constructor) + "' accessible;"
+ "either change its visibility or write a custom InstanceCreator or TypeAdapter for its declaring type: "
+ " either increase its visibility or write a custom InstanceCreator or TypeAdapter for"
// Include the message since it might contain more detailed information
+ exception.getMessage();
+ " its declaring type: " + exception.getMessage();
}
}
@ -132,13 +150,13 @@ public class ReflectionHelper {
}
public static RuntimeException createExceptionForRecordReflectionException(
private static RuntimeException createExceptionForRecordReflectionException(
ReflectiveOperationException exception) {
throw new RuntimeException("Unexpected ReflectiveOperationException occurred"
+ " (Gson " + GsonBuildConfig.VERSION + ")."
+ " To support Java records, reflection is utilized to read out information"
+ " about records. All these invocations happens after it is established"
+ "that records exists in the JVM. This exception is unexpected behaviour.",
+ " that records exist in the JVM. This exception is unexpected behavior.",
exception);
}
@ -164,9 +182,10 @@ public class ReflectionHelper {
private RecordSupportedHelper() throws NoSuchMethodException {
isRecord = Class.class.getMethod("isRecord");
getRecordComponents = Class.class.getMethod("getRecordComponents");
Class<?> recordComponentType = getRecordComponents.getReturnType().getComponentType();
getName = recordComponentType.getMethod("getName");
getType = recordComponentType.getMethod("getType");
// Class java.lang.reflect.RecordComponent
Class<?> classRecordComponent = getRecordComponents.getReturnType().getComponentType();
getName = classRecordComponent.getMethod("getName");
getType = classRecordComponent.getMethod("getType");
}
@Override

View File

@ -17,11 +17,32 @@ package com.google.gson.functional;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;
import com.google.gson.ExclusionStrategy;
import com.google.gson.FieldAttributes;
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.JsonIOException;
import com.google.gson.JsonParseException;
import com.google.gson.JsonPrimitive;
import com.google.gson.JsonSerializationContext;
import com.google.gson.JsonSerializer;
import com.google.gson.ReflectionAccessFilter.FilterResult;
import com.google.gson.TypeAdapter;
import com.google.gson.annotations.Expose;
import com.google.gson.annotations.JsonAdapter;
import com.google.gson.annotations.SerializedName;
import java.util.Objects;
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;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@ -32,32 +53,167 @@ public final class Java17RecordTest {
@Test
public void testFirstNameIsChosenForSerialization() {
MyRecord target = new MyRecord("v1", "v2");
RecordWithCustomNames target = new RecordWithCustomNames("v1", "v2");
// Ensure name1 occurs exactly once, and name2 and name3 don't appear
assertEquals("{\"name\":\"modified-v1\",\"name1\":\"v2\"}", gson.toJson(target));
assertEquals("{\"name\":\"v1\",\"name1\":\"v2\"}", gson.toJson(target));
}
@Test
public void testMultipleNamesDeserializedCorrectly() {
assertEquals("modified-v1", gson.fromJson("{'name':'v1'}", MyRecord.class).a);
assertEquals("v1", gson.fromJson("{'name':'v1'}", RecordWithCustomNames.class).a);
// Both name1 and name2 gets deserialized to b
assertEquals("v11", gson.fromJson("{'name': 'v1', 'name1':'v11'}", MyRecord.class).b);
assertEquals("v2", gson.fromJson("{'name': 'v1', 'name2':'v2'}", MyRecord.class).b);
assertEquals("v3", gson.fromJson("{'name': 'v1', 'name3':'v3'}", MyRecord.class).b);
assertEquals("v11", gson.fromJson("{'name': 'v1', 'name1':'v11'}", RecordWithCustomNames.class).b);
assertEquals("v2", gson.fromJson("{'name': 'v1', 'name2':'v2'}", RecordWithCustomNames.class).b);
assertEquals("v3", gson.fromJson("{'name': 'v1', 'name3':'v3'}", RecordWithCustomNames.class).b);
}
@Test
public void testMultipleNamesInTheSameString() {
// The last value takes precedence
assertEquals("v3",
gson.fromJson("{'name': 'foo', 'name1':'v1','name2':'v2','name3':'v3'}", MyRecord.class).b);
gson.fromJson("{'name': 'foo', 'name1':'v1','name2':'v2','name3':'v3'}", RecordWithCustomNames.class).b);
}
private record RecordWithCustomNames(
@SerializedName("name") String a,
@SerializedName(value = "name1", alternate = {"name2", "name3"}) String b) {}
@Test
public void testSerializedNameOnAccessor() {
record LocalRecord(int i) {
@SerializedName("a")
@Override
public int i() {
return i;
}
}
var exception = assertThrows(JsonIOException.class, () -> gson.getAdapter(LocalRecord.class));
assertEquals("@SerializedName on method '" + LocalRecord.class.getName() + "#i()' is not supported",
exception.getMessage());
}
@Test
public void testFieldNamingStrategy() {
record LocalRecord(int i) {}
Gson gson = new GsonBuilder()
.setFieldNamingStrategy(f -> f.getName() + "-custom")
.create();
assertEquals("{\"i-custom\":1}", gson.toJson(new LocalRecord(1)));
assertEquals(new LocalRecord(2), gson.fromJson("{\"i-custom\":2}", LocalRecord.class));
}
@Test
public void testUnknownJsonProperty() {
record LocalRecord(int i) {}
// Unknown property 'x' should be ignored
assertEquals(new LocalRecord(1), gson.fromJson("{\"i\":1,\"x\":2}", LocalRecord.class));
}
@Test
public void testDuplicateJsonProperties() {
record LocalRecord(Integer a, Integer b) {}
String json = "{\"a\":null,\"a\":2,\"b\":1,\"b\":null}";
// Should use value of last occurrence
assertEquals(new LocalRecord(2, null), gson.fromJson(json, LocalRecord.class));
}
@Test
public void testConstructorRuns() {
assertEquals(new MyRecord(null, null),
gson.fromJson("{'name1': null, 'name2': null}", MyRecord.class));
record LocalRecord(String s) {
LocalRecord {
s = "custom-" + s;
}
}
LocalRecord deserialized = gson.fromJson("{\"s\": null}", LocalRecord.class);
assertEquals(new LocalRecord(null), deserialized);
assertEquals("custom-null", deserialized.s());
}
/** Tests behavior when the canonical constructor throws an exception */
@Test
public void testThrowingConstructor() {
record LocalRecord(String s) {
static final RuntimeException thrownException = new RuntimeException("Custom exception");
@SuppressWarnings("unused")
LocalRecord {
throw thrownException;
}
}
try {
gson.fromJson("{\"s\":\"value\"}", LocalRecord.class);
fail();
}
// TODO: Adjust this once Gson throws more specific exception type
catch (RuntimeException e) {
assertEquals("Failed to invoke constructor '" + LocalRecord.class.getName() + "(String)' with args [value]",
e.getMessage());
assertSame(LocalRecord.thrownException, e.getCause());
}
}
@Test
public void testAccessorIsCalled() {
record LocalRecord(String s) {
@Override
public String s() {
return "accessor-value";
}
}
assertEquals("{\"s\":\"accessor-value\"}", gson.toJson(new LocalRecord(null)));
}
/** Tests behavior when a record accessor method throws an exception */
@Test
public void testThrowingAccessor() {
record LocalRecord(String s) {
static final RuntimeException thrownException = new RuntimeException("Custom exception");
@Override
public String s() {
throw thrownException;
}
}
try {
gson.toJson(new LocalRecord("a"));
fail();
} catch (JsonIOException e) {
assertEquals("Accessor method '" + LocalRecord.class.getName() + "#s()' threw exception",
e.getMessage());
assertSame(LocalRecord.thrownException, e.getCause());
}
}
/** Tests behavior for a record without components */
@Test
public void testEmptyRecord() {
record EmptyRecord() {}
assertEquals("{}", gson.toJson(new EmptyRecord()));
assertEquals(new EmptyRecord(), gson.fromJson("{}", EmptyRecord.class));
}
/**
* Tests behavior when {@code null} is serialized / deserialized as record value;
* basically makes sure the adapter is 'null-safe'
*/
@Test
public void testRecordNull() throws IOException {
record LocalRecord(int i) {}
TypeAdapter<LocalRecord> adapter = gson.getAdapter(LocalRecord.class);
assertEquals("null", adapter.toJson(null));
assertNull(adapter.fromJson("null"));
}
@Test
@ -67,21 +223,208 @@ public final class Java17RecordTest {
}
@Test
public void testPrimitiveNullValues() {
RecordWithPrimitives expected = new RecordWithPrimitives("s", (byte) 0, (short) 0, 0, 0, 0, 0, '\0', false);
// TODO(eamonnmcmanus): consider forbidding null for primitives
String s = "{'aString': 's', 'aByte': null, 'aShort': null, 'anInt': null, 'aLong': null, 'aFloat': null, 'aDouble': null, 'aChar': null, 'aBoolean': null}";
assertEquals(expected, gson.fromJson(s, RecordWithPrimitives.class));
public void testPrimitiveJsonNullValue() {
String s = "{'aString': 's', 'aByte': null, 'aShort': 0}";
var e = assertThrows(JsonParseException.class, () -> gson.fromJson(s, RecordWithPrimitives.class));
assertEquals("null is not allowed as value for record component 'aByte' of primitive type; at path $.aByte",
e.getMessage());
}
public record MyRecord(
@SerializedName("name") String a,
@SerializedName(value = "name1", alternate = {"name2", "name3"}) String b) {
public MyRecord {
a = "modified-" + a;
}
/**
* Tests behavior when JSON contains non-null value, but custom adapter returns null
* for primitive component
*/
@Test
public void testPrimitiveAdapterNullValue() {
Gson gson = new GsonBuilder()
.registerTypeAdapter(byte.class, new TypeAdapter<Byte>() {
@Override public Byte read(JsonReader in) throws IOException {
in.skipValue();
// Always return null
return null;
}
public record RecordWithPrimitives(
@Override public void write(JsonWriter out, Byte value) {
throw new AssertionError("not needed for test");
}
})
.create();
String s = "{'aString': 's', 'aByte': 0}";
var exception = assertThrows(JsonParseException.class, () -> gson.fromJson(s, RecordWithPrimitives.class));
assertEquals("null is not allowed as value for record component 'aByte' of primitive type; at path $.aByte",
exception.getMessage());
}
private record RecordWithPrimitives(
String aString, byte aByte, short aShort, int anInt, long aLong, float aFloat, double aDouble, char aChar, boolean aBoolean) {}
/** Tests behavior when value of Object component is missing; should default to null */
@Test
public void testObjectDefaultValue() {
record LocalRecord(String s, int i) {}
assertEquals(new LocalRecord(null, 1), gson.fromJson("{\"i\":1}", LocalRecord.class));
}
/**
* Tests serialization of a record with {@code static} field.
*
* <p>Important: It is not documented that this is officially supported; this
* test just checks the current behavior.
*/
@Test
public void testStaticFieldSerialization() {
// By default Gson should ignore static fields
assertEquals("{}", gson.toJson(new RecordWithStaticField()));
Gson gson = new GsonBuilder()
// Include static fields
.excludeFieldsWithModifiers(0)
.create();
String json = gson.toJson(new RecordWithStaticField());
assertEquals("{\"s\":\"initial\"}", json);
}
/**
* Tests deserialization of a record with {@code static} field.
*
* <p>Important: It is not documented that this is officially supported; this
* test just checks the current behavior.
*/
@Test
public void testStaticFieldDeserialization() {
// By default Gson should ignore static fields
gson.fromJson("{\"s\":\"custom\"}", RecordWithStaticField.class);
assertEquals("initial", RecordWithStaticField.s);
Gson gson = new GsonBuilder()
// Include static fields
.excludeFieldsWithModifiers(0)
.create();
String oldValue = RecordWithStaticField.s;
try {
RecordWithStaticField obj = gson.fromJson("{\"s\":\"custom\"}", RecordWithStaticField.class);
assertNotNull(obj);
// Currently record deserialization always ignores static fields
assertEquals("initial", RecordWithStaticField.s);
} finally {
RecordWithStaticField.s = oldValue;
}
}
private record RecordWithStaticField() {
static String s = "initial";
}
@Test
public void testExposeAnnotation() {
record RecordWithExpose(
@Expose int a,
int b
) {}
Gson gson = new GsonBuilder().excludeFieldsWithoutExposeAnnotation().create();
String json = gson.toJson(new RecordWithExpose(1, 2));
assertEquals("{\"a\":1}", json);
}
@Test
public void testFieldExclusionStrategy() {
record LocalRecord(int a, int b, double c) {}
Gson gson = new GsonBuilder()
.setExclusionStrategies(new ExclusionStrategy() {
@Override public boolean shouldSkipField(FieldAttributes f) {
return f.getName().equals("a");
}
@Override public boolean shouldSkipClass(Class<?> clazz) {
return clazz == double.class;
}
})
.create();
assertEquals("{\"b\":2}", gson.toJson(new LocalRecord(1, 2, 3.0)));
}
@Test
public void testJsonAdapterAnnotation() {
record Adapter() implements JsonSerializer<String>, JsonDeserializer<String> {
@Override public String deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context) {
return "deserializer-" + json.getAsString();
}
@Override public JsonElement serialize(String src, Type typeOfSrc, JsonSerializationContext context) {
return new JsonPrimitive("serializer-" + src);
}
}
record LocalRecord(
@JsonAdapter(Adapter.class) String s
) {}
assertEquals("{\"s\":\"serializer-a\"}", gson.toJson(new LocalRecord("a")));
assertEquals(new LocalRecord("deserializer-a"), gson.fromJson("{\"s\":\"a\"}", LocalRecord.class));
}
@Test
public void testClassReflectionFilter() {
record Allowed(int a) {}
record Blocked(int b) {}
Gson gson = new GsonBuilder()
.addReflectionAccessFilter(c -> c == Allowed.class ? FilterResult.ALLOW : FilterResult.BLOCK_ALL)
.create();
String json = gson.toJson(new Allowed(1));
assertEquals("{\"a\":1}", json);
var exception = assertThrows(JsonIOException.class, () -> gson.toJson(new Blocked(1)));
assertEquals("ReflectionAccessFilter does not permit using reflection for class " + Blocked.class.getName() +
". Register a TypeAdapter for this type or adjust the access filter.",
exception.getMessage());
}
@Test
public void testReflectionFilterBlockInaccessible() {
Gson gson = new GsonBuilder()
.addReflectionAccessFilter(c -> FilterResult.BLOCK_INACCESSIBLE)
.create();
var exception = assertThrows(JsonIOException.class, () -> gson.toJson(new PrivateRecord(1)));
assertEquals("Constructor 'com.google.gson.functional.Java17RecordTest$PrivateRecord(int)' is not accessible and"
+ " ReflectionAccessFilter does not permit making it accessible. Register a TypeAdapter for the declaring"
+ " type, adjust the access filter or increase the visibility of the element and its declaring type.",
exception.getMessage());
exception = assertThrows(JsonIOException.class, () -> gson.fromJson("{}", PrivateRecord.class));
assertEquals("Constructor 'com.google.gson.functional.Java17RecordTest$PrivateRecord(int)' is not accessible and"
+ " ReflectionAccessFilter does not permit making it accessible. Register a TypeAdapter for the declaring"
+ " type, adjust the access filter or increase the visibility of the element and its declaring type.",
exception.getMessage());
assertEquals("{\"i\":1}", gson.toJson(new PublicRecord(1)));
assertEquals(new PublicRecord(2), gson.fromJson("{\"i\":2}", PublicRecord.class));
}
private record PrivateRecord(int i) {}
public record PublicRecord(int i) {}
/**
* Tests behavior when {@code java.lang.Record} is used as type for serialization
* and deserialization.
*/
@Test
public void testRecordBaseClass() {
record LocalRecord(int i) {}
assertEquals("{}", gson.toJson(new LocalRecord(1), Record.class));
var exception = assertThrows(JsonIOException.class, () -> gson.fromJson("{}", Record.class));
assertEquals("Abstract classes can't be instantiated! Register an InstanceCreator or a TypeAdapter for"
+ " this type. Class name: java.lang.Record",
exception.getMessage());
}
}

View File

@ -20,6 +20,7 @@ import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.InstanceCreator;
import com.google.gson.JsonElement;
import com.google.gson.JsonIOException;
import com.google.gson.JsonObject;
import com.google.gson.JsonParseException;
import com.google.gson.JsonSerializationContext;
@ -482,6 +483,16 @@ public class ObjectTest extends TestCase {
gson.fromJson(gson.toJson(product), Product.class);
}
static final class Department {
public String name = "abc";
public String code = "123";
}
static final class Product {
private List<String> attributes = new ArrayList<>();
private List<Department> departments = new ArrayList<>();
}
// http://code.google.com/p/google-gson/issues/detail?id=270
public void testDateAsMapObjectField() {
HasObjectMap a = new HasObjectMap();
@ -493,17 +504,92 @@ public class ObjectTest extends TestCase {
}
}
public class HasObjectMap {
static class HasObjectMap {
Map<String, Object> map = new HashMap<>();
}
static final class Department {
public String name = "abc";
public String code = "123";
/**
* Tests serialization of a class with {@code static} field.
*
* <p>Important: It is not documented that this is officially supported; this
* test just checks the current behavior.
*/
public void testStaticFieldSerialization() {
// By default Gson should ignore static fields
assertEquals("{}", gson.toJson(new ClassWithStaticField()));
Gson gson = new GsonBuilder()
// Include static fields
.excludeFieldsWithModifiers(0)
.create();
String json = gson.toJson(new ClassWithStaticField());
assertEquals("{\"s\":\"initial\"}", json);
json = gson.toJson(new ClassWithStaticFinalField());
assertEquals("{\"s\":\"initial\"}", json);
}
static final class Product {
private List<String> attributes = new ArrayList<>();
private List<Department> departments = new ArrayList<>();
/**
* Tests deserialization of a class with {@code static} field.
*
* <p>Important: It is not documented that this is officially supported; this
* test just checks the current behavior.
*/
public void testStaticFieldDeserialization() {
// By default Gson should ignore static fields
gson.fromJson("{\"s\":\"custom\"}", ClassWithStaticField.class);
assertEquals("initial", ClassWithStaticField.s);
Gson gson = new GsonBuilder()
// Include static fields
.excludeFieldsWithModifiers(0)
.create();
String oldValue = ClassWithStaticField.s;
try {
ClassWithStaticField obj = gson.fromJson("{\"s\":\"custom\"}", ClassWithStaticField.class);
assertNotNull(obj);
assertEquals("custom", ClassWithStaticField.s);
} finally {
ClassWithStaticField.s = oldValue;
}
try {
gson.fromJson("{\"s\":\"custom\"}", ClassWithStaticFinalField.class);
fail();
} catch (JsonIOException e) {
assertEquals("Cannot set value of 'static final' field 'com.google.gson.functional.ObjectTest$ClassWithStaticFinalField#s'",
e.getMessage());
}
}
static class ClassWithStaticField {
static String s = "initial";
}
static class ClassWithStaticFinalField {
static final String s = "initial";
}
public void testThrowingDefaultConstructor() {
try {
gson.fromJson("{}", ClassWithThrowingConstructor.class);
fail();
}
// TODO: Adjust this once Gson throws more specific exception type
catch (RuntimeException e) {
assertEquals("Failed to invoke constructor 'com.google.gson.functional.ObjectTest$ClassWithThrowingConstructor()' with no args",
e.getMessage());
assertSame(ClassWithThrowingConstructor.thrownException, e.getCause());
}
}
static class ClassWithThrowingConstructor {
static final RuntimeException thrownException = new RuntimeException("Custom exception");
public ClassWithThrowingConstructor() {
throw thrownException;
}
}
}

View File

@ -54,7 +54,8 @@ public class ReflectionAccessFilterTest {
// Note: This test is rather brittle and depends on the JDK implementation
assertEquals(
"Field 'java.io.File#path' is not accessible and ReflectionAccessFilter does not permit"
+ "making it accessible. Register a TypeAdapter for the declaring type or adjust the access filter.",
+ " making it accessible. Register a TypeAdapter for the declaring type, adjust the access"
+ " filter or increase the visibility of the element and its declaring type.",
expected.getMessage()
);
}
@ -86,7 +87,8 @@ public class ReflectionAccessFilterTest {
} catch (JsonIOException expected) {
assertEquals(
"Field 'java.io.Reader#lock' is not accessible and ReflectionAccessFilter does not permit"
+ "making it accessible. Register a TypeAdapter for the declaring type or adjust the access filter.",
+ " making it accessible. Register a TypeAdapter for the declaring type, adjust the access"
+ " filter or increase the visibility of the element and its declaring type.",
expected.getMessage()
);
}
@ -154,7 +156,8 @@ public class ReflectionAccessFilterTest {
assertEquals(
"Field 'com.google.gson.functional.ReflectionAccessFilterTest$ClassWithStaticField#i'"
+ " is not accessible and ReflectionAccessFilter does not permit making it accessible."
+ "Register a TypeAdapter for the declaring type or adjust the access filter.",
+ " Register a TypeAdapter for the declaring type, adjust the access filter or increase"
+ " the visibility of the element and its declaring type.",
expected.getMessage()
);
}
@ -235,7 +238,8 @@ public class ReflectionAccessFilterTest {
assertEquals(
"Field 'com.google.gson.functional.ReflectionAccessFilterTest$ClassWithPrivateField#i'"
+ " is not accessible and ReflectionAccessFilter does not permit making it accessible."
+ "Register a TypeAdapter for the declaring type or adjust the access filter.",
+ " Register a TypeAdapter for the declaring type, adjust the access filter or increase"
+ " the visibility of the element and its declaring type.",
expected.getMessage()
);
}
@ -322,7 +326,7 @@ public class ReflectionAccessFilterTest {
}
@Override public void write(JsonWriter out, ClassWithoutNoArgsConstructor value) throws IOException {
throw new AssertionError("Not needed for test");
};
}
})
.create();
ClassWithoutNoArgsConstructor deserialized = gson.fromJson("{}", ClassWithoutNoArgsConstructor.class);

View File

@ -119,8 +119,8 @@ public class ReflectionAccessTest {
fail("Unexpected exception; test has to be run with `--illegal-access=deny`");
} catch (JsonIOException expected) {
assertTrue(expected.getMessage().startsWith(
"Failed making constructor 'java.util.Collections$EmptyList#EmptyList()' accessible; "
+ "either change its visibility or write a custom InstanceCreator or TypeAdapter for its declaring type"
"Failed making constructor 'java.util.Collections$EmptyList()' accessible;"
+ " either increase its visibility or write a custom InstanceCreator or TypeAdapter for its declaring type: "
));
}
}

View File

@ -6,42 +6,39 @@ import static org.junit.Assert.assertNotEquals;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.TypeAdapter;
import com.google.gson.internal.reflect.ReflectionHelperTest;
import com.google.gson.internal.reflect.Java17ReflectionHelperTest;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
import java.io.IOException;
import java.nio.file.attribute.GroupPrincipal;
import java.nio.file.attribute.UserPrincipal;
import java.security.Principal;
import org.junit.AssumptionViolatedException;
import org.junit.Before;
import org.junit.Test;
public class ReflectiveTypeAdapterFactoryTest {
public class Java17ReflectiveTypeAdapterFactoryTest {
// The class jdk.net.UnixDomainPrincipal is one of the few Record types that are included in the
// JDK.
// The class jdk.net.UnixDomainPrincipal is one of the few Record types that are included in the JDK.
// We use this to test serialization and deserialization of Record classes, so we do not need to
// have
// record support at the language level for these tests. This class was added in JDK 16.
// have record support at the language level for these tests. This class was added in JDK 16.
Class<?> unixDomainPrincipalClass;
@Before
public void setUp() throws Exception {
try {
Class.forName("java.lang.Record");
unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal");
} catch (ClassNotFoundException e) {
// Records not supported, ignore
throw new AssumptionViolatedException("java.lang.Record not supported");
}
// Class for which the normal reflection based adapter is used
private static class DummyClass {
@SuppressWarnings("unused")
public String s;
}
@Test
public void testCustomAdapterForRecords() {
Gson gson = new Gson();
TypeAdapter<?> recordAdapter = gson.getAdapter(unixDomainPrincipalClass);
TypeAdapter<?> defaultReflectionAdapter = gson.getAdapter(UserPrincipal.class);
TypeAdapter<?> defaultReflectionAdapter = gson.getAdapter(DummyClass.class);
assertNotEquals(recordAdapter.getClass(), defaultReflectionAdapter.getClass());
}
@ -77,7 +74,7 @@ public class ReflectiveTypeAdapterFactoryTest {
final String name = in.nextString();
// This type adapter is only used for Group and User Principal, both of which are implemented by PrincipalImpl.
@SuppressWarnings("unchecked")
T principal = (T) new ReflectionHelperTest.PrincipalImpl(name);
T principal = (T) new Java17ReflectionHelperTest.PrincipalImpl(name);
return principal;
}
}

View File

@ -11,22 +11,9 @@ import java.lang.reflect.Method;
import java.nio.file.attribute.GroupPrincipal;
import java.nio.file.attribute.UserPrincipal;
import java.util.Objects;
import org.junit.AssumptionViolatedException;
import org.junit.Before;
import org.junit.Test;
public class ReflectionHelperTest {
@Before
public void setUp() throws Exception {
try {
Class.forName("java.lang.Record");
} catch (ClassNotFoundException e) {
// Records not supported, ignore
throw new AssumptionViolatedException("java.lang.Record not supported");
}
}
public class Java17ReflectionHelperTest {
@Test
public void testJava17Record() throws ClassNotFoundException {
Class<?> unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal");
@ -54,8 +41,11 @@ public class ReflectionHelperTest {
Object unixDomainPrincipal =
ReflectionHelper.getCanonicalRecordConstructor(unixDomainPrincipalClass)
.newInstance(new PrincipalImpl("user"), new PrincipalImpl("group"));
for (String componentName :
ReflectionHelper.getRecordComponentNames(unixDomainPrincipalClass)) {
String[] componentNames = ReflectionHelper.getRecordComponentNames(unixDomainPrincipalClass);
assertTrue(componentNames.length > 0);
for (String componentName : componentNames) {
Field componentField = unixDomainPrincipalClass.getDeclaredField(componentName);
Method accessor = ReflectionHelper.getAccessor(unixDomainPrincipalClass, componentField);
Object principal = accessor.invoke(unixDomainPrincipal);