diff --git a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java index da330c30..115a2a09 100644 --- a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java +++ b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java @@ -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); } diff --git a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java index 3da14f23..5ddac50e 100644 --- a/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java +++ b/gson/src/main/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactory.java @@ -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 adapter = (TypeAdapter) new RecordAdapter<>(raw, + getBoundFields(gson, type, raw, blockInaccessible, true), blockInaccessible); + return adapter; } ObjectConstructor 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 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 typeAdapter = (TypeAdapter) 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) { - checkAccessible(source, field); + 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,9 +241,9 @@ 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 " - + "or adjust the access filter."); + 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())) { - accessor = ReflectionHelper.getAccessor(raw, field); + 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); + } + + // @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. 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) { + // 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 type of objects that this Adapter creates. * @param 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 extends TypeAdapter { - protected final Map boundFields; + final Map boundFields; - protected Adapter(Map boundFields) { + Adapter(Map 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 extends Adapter { - static Map, Object> PRIMITIVE_DEFAULTS = primitiveDefaults(); + static final Map, Object> PRIMITIVE_DEFAULTS = primitiveDefaults(); - // The actual record constructor. - private final Constructor constructor; + // The canonical constructor of the record + private final Constructor 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 componentIndices = new HashMap<>(); - RecordAdapter(Class raw, Map boundFields) { + RecordAdapter(Class raw, Map boundFields, boolean blockInaccessible) { super(boundFields); - this.constructor = ReflectionHelper.getCanonicalRecordConstructor(raw); - // Ensure the constructor is accessible - ReflectionHelper.makeAccessible(this.constructor); + constructor = ReflectionHelper.getCanonicalRecordConstructor(raw); + + if (blockInaccessible) { + checkAccessible(null, constructor); + } else { + // Ensure the constructor is accessible + 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()); } } } diff --git a/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java b/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java index 80df515a..ac061212 100644 --- a/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java +++ b/gson/src/main/java/com/google/gson/internal/reflect/ReflectionHelper.java @@ -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); } } /** - * Creates a string representation for a constructor. - * E.g.: {@code java.lang.String#String(char[], int, int)} + * 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 */ - 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 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 = " " + 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(char[], int, int)} + */ + 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(')'); } /** @@ -98,10 +116,10 @@ public class ReflectionHelper { constructor.setAccessible(true); 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: " + return "Failed making constructor '" + constructorToString(constructor) + "' accessible;" + + " 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(); } } @@ -125,20 +143,20 @@ public class ReflectionHelper { public static RuntimeException createExceptionForUnexpectedIllegalAccess( IllegalAccessException exception) { - throw new RuntimeException("Unexpected IllegalAccessException occurred (Gson " + GsonBuildConfig.VERSION + "). " - + "Certain ReflectionAccessFilter features require Java >= 9 to work correctly. If you are not using " - + "ReflectionAccessFilter, report this to the Gson maintainers.", + throw new RuntimeException("Unexpected IllegalAccessException occurred (Gson " + GsonBuildConfig.VERSION + ")." + + " Certain ReflectionAccessFilter features require Java >= 9 to work correctly. If you are not using" + + " ReflectionAccessFilter, report this to the Gson maintainers.", exception); } - 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.", + 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 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 diff --git a/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java b/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java index 023bec30..a172f5a4 100644 --- a/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java +++ b/gson/src/test/java/com/google/gson/functional/Java17RecordTest.java @@ -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 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() { + @Override public Byte read(JsonReader in) throws IOException { + in.skipValue(); + // Always return null + return null; + } + + @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. + * + *

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. + * + *

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; } } - public record RecordWithPrimitives( - String aString, byte aByte, short aShort, int anInt, long aLong, float aFloat, double aDouble, char aChar, boolean aBoolean) {} + 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, JsonDeserializer { + @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()); + } } diff --git a/gson/src/test/java/com/google/gson/functional/ObjectTest.java b/gson/src/test/java/com/google/gson/functional/ObjectTest.java index de384879..bed5b598 100644 --- a/gson/src/test/java/com/google/gson/functional/ObjectTest.java +++ b/gson/src/test/java/com/google/gson/functional/ObjectTest.java @@ -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 attributes = new ArrayList<>(); + private List 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 map = new HashMap<>(); } - static final class Department { - public String name = "abc"; - public String code = "123"; + /** + * Tests serialization of a class with {@code static} field. + * + *

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 attributes = new ArrayList<>(); - private List departments = new ArrayList<>(); + /** + * Tests deserialization of a class with {@code static} field. + * + *

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; + } } } diff --git a/gson/src/test/java/com/google/gson/functional/ReflectionAccessFilterTest.java b/gson/src/test/java/com/google/gson/functional/ReflectionAccessFilterTest.java index 8814b377..6c9ab449 100644 --- a/gson/src/test/java/com/google/gson/functional/ReflectionAccessFilterTest.java +++ b/gson/src/test/java/com/google/gson/functional/ReflectionAccessFilterTest.java @@ -53,8 +53,9 @@ public class ReflectionAccessFilterTest { } catch (JsonIOException expected) { // 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.", + "Field 'java.io.File#path' 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.", expected.getMessage() ); } @@ -85,8 +86,9 @@ public class ReflectionAccessFilterTest { fail("Expected exception; test needs to be run with Java >= 9"); } 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.", + "Field 'java.io.Reader#lock' 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.", expected.getMessage() ); } @@ -104,8 +106,8 @@ public class ReflectionAccessFilterTest { fail(); } catch (JsonIOException expected) { assertEquals( - "ReflectionAccessFilter does not permit using reflection for class java.lang.Thread. " - + "Register a TypeAdapter for this type or adjust the access filter.", + "ReflectionAccessFilter does not permit using reflection for class java.lang.Thread." + + " Register a TypeAdapter for this type or adjust the access filter.", expected.getMessage() ); } @@ -122,9 +124,9 @@ public class ReflectionAccessFilterTest { fail(); } catch (JsonIOException expected) { assertEquals( - "ReflectionAccessFilter does not permit using reflection for class java.io.Reader " - + "(supertype of class com.google.gson.functional.ReflectionAccessFilterTest$ClassExtendingJdkClass). " - + "Register a TypeAdapter for this type or adjust the access filter.", + "ReflectionAccessFilter does not permit using reflection for class java.io.Reader" + + " (supertype of class com.google.gson.functional.ReflectionAccessFilterTest$ClassExtendingJdkClass)." + + " Register a TypeAdapter for this type or adjust the access filter.", expected.getMessage() ); } @@ -152,9 +154,10 @@ public class ReflectionAccessFilterTest { fail("Expected exception; test needs to be run with Java >= 9"); } catch (JsonIOException expected) { 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.", + "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, adjust the access filter or increase" + + " the visibility of the element and its declaring type.", expected.getMessage() ); } @@ -194,9 +197,9 @@ public class ReflectionAccessFilterTest { fail(); } catch (JsonIOException expected) { assertEquals( - "ReflectionAccessFilter does not permit using reflection for class " - + "com.google.gson.functional.ReflectionAccessFilterTest$SuperTestClass. " - + "Register a TypeAdapter for this type or adjust the access filter.", + "ReflectionAccessFilter does not permit using reflection for class" + + " com.google.gson.functional.ReflectionAccessFilterTest$SuperTestClass." + + " Register a TypeAdapter for this type or adjust the access filter.", expected.getMessage() ); } @@ -233,9 +236,10 @@ public class ReflectionAccessFilterTest { fail("Expected exception; test needs to be run with Java >= 9"); } catch (JsonIOException expected) { 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.", + "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, adjust the access filter or increase" + + " the visibility of the element and its declaring type.", expected.getMessage() ); } @@ -274,9 +278,9 @@ public class ReflectionAccessFilterTest { fail("Expected exception; test needs to be run with Java >= 9"); } catch (JsonIOException expected) { assertEquals( - "Unable to invoke no-args constructor of class com.google.gson.functional.ReflectionAccessFilterTest$ClassWithPrivateNoArgsConstructor; " - + "constructor is not accessible and ReflectionAccessFilter does not permit making it accessible. Register an " - + "InstanceCreator or a TypeAdapter for this type, change the visibility of the constructor or adjust the access filter.", + "Unable to invoke no-args constructor of class com.google.gson.functional.ReflectionAccessFilterTest$ClassWithPrivateNoArgsConstructor;" + + " constructor is not accessible and ReflectionAccessFilter does not permit making it accessible. Register an" + + " InstanceCreator or a TypeAdapter for this type, change the visibility of the constructor or adjust the access filter.", expected.getMessage() ); } @@ -306,9 +310,9 @@ public class ReflectionAccessFilterTest { fail(); } catch (JsonIOException expected) { assertEquals( - "Unable to create instance of class com.google.gson.functional.ReflectionAccessFilterTest$ClassWithoutNoArgsConstructor; " - + "ReflectionAccessFilter does not permit using reflection or Unsafe. Register an InstanceCreator " - + "or a TypeAdapter for this type or adjust the access filter to allow using reflection.", + "Unable to create instance of class com.google.gson.functional.ReflectionAccessFilterTest$ClassWithoutNoArgsConstructor;" + + " ReflectionAccessFilter does not permit using reflection or Unsafe. Register an InstanceCreator" + + " or a TypeAdapter for this type or adjust the access filter to allow using reflection.", 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); @@ -368,8 +372,8 @@ public class ReflectionAccessFilterTest { fail(); } catch (JsonIOException expected) { assertEquals( - "ReflectionAccessFilter does not permit using reflection for class com.google.gson.functional.ReflectionAccessFilterTest$OtherClass. " - + "Register a TypeAdapter for this type or adjust the access filter.", + "ReflectionAccessFilter does not permit using reflection for class com.google.gson.functional.ReflectionAccessFilterTest$OtherClass." + + " Register a TypeAdapter for this type or adjust the access filter.", expected.getMessage() ); } @@ -428,8 +432,8 @@ public class ReflectionAccessFilterTest { fail(); } catch (JsonIOException expected) { assertEquals( - "Interfaces can't be instantiated! Register an InstanceCreator or a TypeAdapter for " - + "this type. Interface name: java.lang.Runnable", + "Interfaces can't be instantiated! Register an InstanceCreator or a TypeAdapter for" + + " this type. Interface name: java.lang.Runnable", expected.getMessage() ); } diff --git a/gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java b/gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java index cdf56852..02649c5f 100644 --- a/gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java +++ b/gson/src/test/java/com/google/gson/functional/ReflectionAccessTest.java @@ -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: " )); } } diff --git a/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java b/gson/src/test/java/com/google/gson/internal/bind/Java17ReflectiveTypeAdapterFactoryTest.java similarity index 77% rename from gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java rename to gson/src/test/java/com/google/gson/internal/bind/Java17ReflectiveTypeAdapterFactoryTest.java index 8ee15aa0..18984c7b 100644 --- a/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java +++ b/gson/src/test/java/com/google/gson/internal/bind/Java17ReflectiveTypeAdapterFactoryTest.java @@ -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"); - } + unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); + } + + // 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; } } diff --git a/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java b/gson/src/test/java/com/google/gson/internal/reflect/Java17ReflectionHelperTest.java similarity index 84% rename from gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java rename to gson/src/test/java/com/google/gson/internal/reflect/Java17ReflectionHelperTest.java index f5c827e8..4d4089e8 100644 --- a/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java +++ b/gson/src/test/java/com/google/gson/internal/reflect/Java17ReflectionHelperTest.java @@ -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);