From 198956249ec9233b48209bb98bb032df8731e7cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A5le=20Undheim?= Date: Thu, 22 Sep 2022 12:52:19 +0200 Subject: [PATCH 1/4] Support Java Records when present in JVM. Fixes google/gson#1794 Added support in the ReflectionHelper to detect if a class is a record on the JVM (via reflection), and if so, we will create a special RecordAdapter to deserialize records, using the canoncial constructor. The ReflectionTypeAdapterFactory had to be refactored a bit to support this. The Adapter class inside the factory is now abstract, with concrete implementations for normal field reflection and for Records. The common code is in the Adapter, with each implementation deserializing values into an intermediary object. For the FieldReflectionAdapter, the intermediary is actually the final result, and field access is used to write to fields as before. For the RecordAdapter the intermediary is the Object[] to pass to the Record constructor. --- .../bind/ReflectiveTypeAdapterFactory.java | 184 +++++++++++++++--- .../internal/reflect/ReflectionHelper.java | 146 +++++++++++++- 2 files changed, 300 insertions(+), 30 deletions(-) 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 31a44e1a8a..d95087691e 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 @@ -38,12 +38,16 @@ import com.google.gson.stream.JsonToken; import com.google.gson.stream.JsonWriter; import java.io.IOException; +import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.lang.reflect.Type; +import java.lang.reflect.Array; +import java.util.Arrays; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -94,22 +98,32 @@ private List getFieldNames(Field f) { return fieldNames; } - @Override public TypeAdapter create(Gson gson, final TypeToken type) { + @Override + public TypeAdapter create(Gson gson, final TypeToken type) { Class raw = type.getRawType(); if (!Object.class.isAssignableFrom(raw)) { return null; // it's a primitive! } - FilterResult filterResult = ReflectionAccessFilterHelper.getFilterResult(reflectionFilters, raw); + if (ReflectionHelper.isRecord(raw)) { + // We can assume blockInaccessible here, because it will never kick inn. We are not doing + // direct field access, and are instead constructing records via their canonical constructor. + return new RecordAdapter<>(raw, getBoundFields(gson, type, raw, true)); + } + + FilterResult filterResult = + ReflectionAccessFilterHelper.getFilterResult(reflectionFilters, raw); if (filterResult == FilterResult.BLOCK_ALL) { - throw new JsonIOException("ReflectionAccessFilter does not permit using reflection for " - + raw + ". Register a TypeAdapter for this type or adjust the access filter."); + throw new JsonIOException( + "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; ObjectConstructor constructor = constructorConstructor.get(type); - return new Adapter<>(constructor, getBoundFields(gson, type, raw, blockInaccessible)); + return new FieldReflectionAdapter<>(constructor, getBoundFields(gson, type, raw, blockInaccessible)); } private static void checkAccessible(Object object, Field field) { @@ -156,7 +170,14 @@ private ReflectiveTypeAdapterFactory.BoundField createBoundField( : new TypeAdapterRuntimeTypeWrapper<>(context, typeAdapter, fieldType.getType()); t.write(writer, fieldValue); } - @Override void read(JsonReader reader, Object value) + + @Override + Object read(JsonReader reader) throws IOException { + return typeAdapter.read(reader); + } + + @Override + void read(JsonReader reader, Object value) throws IOException, IllegalAccessException { Object fieldValue = typeAdapter.read(reader); if (fieldValue != null || !isPrimitive) { @@ -234,26 +255,59 @@ protected BoundField(String name, boolean serialized, boolean deserialized) { this.serialized = serialized; this.deserialized = deserialized; } + abstract void write(JsonWriter writer, Object value) throws IOException, IllegalAccessException; + + abstract Object read(JsonReader reader) throws IOException; + abstract void read(JsonReader reader, Object value) throws IOException, IllegalAccessException; } - public static final class Adapter extends TypeAdapter { - private final ObjectConstructor constructor; - private final Map boundFields; + /** + * Base class for Adapters produced by this factory. + * + *

The {@link RecordAdapter} is a special case to handle Java 17 and later records, for all + * other types we use the {@link FieldReflectionAdapter}. This class encapuslates the common logic + * for serialization and deserialization. During deserialization, we construct an intermediary + * ResultHolder R, which we append values to. After the object has been read in full, the {@link + * #finalize(Object)} method is used to convert the intermediary to an instance of T. + * + * @param type of objects that this Adapter creates. + * @param intermediary type used to build the deserialization result. + */ + abstract static class Adapter extends TypeAdapter { + protected final Map boundFields; - Adapter(ObjectConstructor constructor, Map boundFields) { - this.constructor = constructor; + protected Adapter(Map boundFields) { this.boundFields = boundFields; } - @Override public T read(JsonReader in) throws IOException { + @Override + public void write(JsonWriter out, T value) throws IOException { + if (value == null) { + out.nullValue(); + return; + } + + out.beginObject(); + try { + for (BoundField boundField : boundFields.values()) { + boundField.write(out, value); + } + } catch (IllegalAccessException e) { + throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e); + } + out.endObject(); + } + + @Override + public T read(JsonReader in) throws IOException { if (in.peek() == JsonToken.NULL) { in.nextNull(); return null; } - T instance = constructor.construct(); + I intermediary = createIntermediary(); try { in.beginObject(); @@ -263,7 +317,7 @@ public static final class Adapter extends TypeAdapter { if (field == null || !field.deserialized) { in.skipValue(); } else { - field.read(in, instance); + readField(intermediary, in, field); } } } catch (IllegalStateException e) { @@ -272,24 +326,102 @@ public static final class Adapter extends TypeAdapter { throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e); } in.endObject(); - return instance; + return finalize(intermediary); } - @Override public void write(JsonWriter out, T value) throws IOException { - if (value == null) { - out.nullValue(); - return; + // Create the Object that will be used to collect each field value + abstract I createIntermediary(); + // Read a single Bounded field into the intermediary. The JsonReader will be pointed at the + // start of the value + // for the BoundField to read from. + abstract void readField(I intermediary, JsonReader in, BoundField field) + throws IllegalAccessException, IOException; + // Convert the intermediary to a final instance of T. + abstract T finalize(I intermediary); + } + + private static final class FieldReflectionAdapter extends Adapter { + private final ObjectConstructor constructor; + + FieldReflectionAdapter(ObjectConstructor constructor, Map boundFields) { + super(boundFields); + this.constructor = constructor; + } + + @Override + T createIntermediary() { + return constructor.construct(); + } + + @Override + void readField(T intermediary, JsonReader in, BoundField field) + throws IllegalAccessException, IOException { + field.read(in, intermediary); + } + + @Override + T finalize(T intermediary) { + return intermediary; + } + } + + private static final class RecordAdapter extends Adapter { + // The actual record constructor. + private final Constructor constructor; + // Array of arguments to the constructor, initialized with default values for primitives + private final Object[] constructorArgsDefaults; + // Map from field names to index into the constructors arguments. + private final Map fieldIndices = new HashMap<>(); + + RecordAdapter(Class raw, Map boundFields) { + super(boundFields); + this.constructor = ReflectionHelper.getCanonicalRecordConstructor(raw); + String[] recordFields = ReflectionHelper.recordFields(raw); + for (int i = 0; i < recordFields.length; i++) { + fieldIndices.put(recordFields[i], i); + } + Class[] parameterTypes = constructor.getParameterTypes(); + constructorArgsDefaults = new Object[parameterTypes.length]; + for (int i = 0; i < parameterTypes.length; i++) { + if (parameterTypes[i].isPrimitive()) { + constructorArgsDefaults[i] = Array.get(Array.newInstance(parameterTypes[i], 1), 0); + } } + } - out.beginObject(); + @Override + Object[] createIntermediary() { + Object[] constructorArgs = new Object[constructorArgsDefaults.length]; + System.arraycopy(constructorArgsDefaults, 0, constructorArgs, 0, constructorArgs.length); + return constructorArgs; + } + + @Override + void readField(Object[] intermediary, JsonReader in, BoundField field) throws IOException { + Integer fieldIndex = fieldIndices.get(field.name); + if (fieldIndex == 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" + + " 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."); + } + intermediary[fieldIndex] = field.read(in); + } + + @Override + @SuppressWarnings("unchecked") + T finalize(Object[] intermediary) { try { - for (BoundField boundField : boundFields.values()) { - boundField.write(out, value); - } - } catch (IllegalAccessException e) { - throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e); + return (T) constructor.newInstance(intermediary); + } catch (ReflectiveOperationException e) { + throw new RuntimeException( + "Failed to invoke " + constructor + " with args " + Arrays.toString(intermediary), e); } - out.endObject(); } } } 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 97230ff6f5..f8f29fc70d 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 @@ -2,15 +2,32 @@ import com.google.gson.JsonIOException; import com.google.gson.internal.GsonBuildConfig; +import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; public class ReflectionHelper { - private ReflectionHelper() { } + + private static final RecordHelper RECORD_HELPER; + + static { + RecordHelper instance; + try { + // Try to construct the RecordSupportedHelper, if this fails, records are not supported on this JVM. + instance = new RecordSupportedHelper(); + } catch (NoSuchMethodException e) { + instance = new RecordNotSupportedHelper(); + } + RECORD_HELPER = instance; + } + + private ReflectionHelper() {} /** - * Tries making the field accessible, wrapping any thrown exception in a - * {@link JsonIOException} with descriptive message. + * 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 @@ -65,10 +82,131 @@ public static String tryMakeAccessible(Constructor constructor) { } } - public static RuntimeException createExceptionForUnexpectedIllegalAccess(IllegalAccessException exception) { + public static boolean isRecord(Class raw) { + return RECORD_HELPER.isRecord(raw); + } + + public static String[] recordFields(Class raw) { + return RECORD_HELPER.recordFields(raw); + } + + 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.", exception); } + + public static Constructor getCanonicalRecordConstructor(Class raw) { + return RECORD_HELPER.getCanonicalRecordConstructor(raw); + } + + /** + * Internal abstraction over reflection when Records are supported. + */ + private abstract static class RecordHelper { + abstract boolean isRecord(Class clazz); + + abstract String[] recordFields(Class clazz); + + abstract Constructor getCanonicalRecordConstructor(Class raw); + } + + private static class RecordSupportedHelper extends RecordHelper { + private final Method isRecord; + private final Method getRecordComponents; + private final Method getName; + private final Method getType; + + private RecordSupportedHelper() throws NoSuchMethodException { + isRecord = Class.class.getMethod("isRecord"); + getRecordComponents = Class.class.getDeclaredMethod("getRecordComponents"); + Class recordComponentType = getRecordComponents.getReturnType().getComponentType(); + getName = recordComponentType.getDeclaredMethod("getName"); + getType = recordComponentType.getDeclaredMethod("getType"); + } + + @Override + boolean isRecord(Class raw) { + try { + return boolean.class.cast(isRecord.invoke(raw)); + } catch (IllegalAccessException e) { + throw createExceptionForUnexpectedIllegalAccess(e); + } catch (InvocationTargetException e) { + throw new RuntimeException("Failed to reflect into record components on" + + " class [" + raw + "], this is unexpected behaviour, as these methods should" + + " not throw when they are defined on Class", + e + ); + } + } + + @Override + String[] recordFields(Class raw) { + try { + Object recordComponents = getRecordComponents.invoke(raw); + int componentCount = Array.getLength(recordComponents); + String[] recordFields = new String[componentCount]; + for (int i = 0; i < componentCount; i++) { + recordFields[i] = + String.class.cast(getName.invoke(Array.get(recordComponents, i))); + } + return recordFields; + } catch (IllegalAccessException e) { + throw createExceptionForUnexpectedIllegalAccess(e); + } catch (ReflectiveOperationException e) { + throw new RuntimeException("Failed to reflect into record components on" + + " class [" + raw + "], this is unexpected behaviour, as these methods should" + + " not throw when they are defined on Class", + e + ); + } + } + + @Override + public Constructor getCanonicalRecordConstructor(Class raw) { + try { + Object recordComponents = getRecordComponents.invoke(raw); + int componentCount = Array.getLength(recordComponents); + Class[] recordFields = new Class[componentCount]; + for (int i = 0; i < componentCount; i++) { + recordFields[i] = + Class.class.cast(getType.invoke(Array.get(recordComponents, i))); + } + return raw.getConstructor(recordFields); + } catch (IllegalAccessException e) { + throw createExceptionForUnexpectedIllegalAccess(e); + } catch (ReflectiveOperationException e) { + throw new RuntimeException("Failed to reflect into record components on" + + " class [" + raw + "], this is unexpected behaviour, as these methods should" + + " not throw when they are defined on Class", + e + ); + } + } + } + + /** + * Instance used when records are not supported + */ + private static class RecordNotSupportedHelper extends RecordHelper { + + @Override + boolean isRecord(Class clazz) { + return false; + } + + @Override + String[] recordFields(Class clazz) { + throw new UnsupportedOperationException( + "Records are not supported on this JVM, this method should not be called"); + } + + @Override + Constructor getCanonicalRecordConstructor(Class raw) { + throw new UnsupportedOperationException( + "Records are not supported on this JVM, this method should not be called"); + } + } } From d12141e9f668c077907d430bcd4164d31ef4e283 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A5le=20Undheim?= Date: Mon, 26 Sep 2022 22:26:25 +0200 Subject: [PATCH 2/4] Fixed comments from @Marcono1234 Also updated so that we now use the record accessor method to read out values from a record, so that direct field access is not necessary. Also added some tests, that should only execute on Java versions with record support, and be ignored for other JVMs --- .../bind/ReflectiveTypeAdapterFactory.java | 157 +++++++++++------- .../internal/reflect/ReflectionHelper.java | 111 +++++++------ .../ReflectiveTypeAdapterFactoryTest.java | 132 +++++++++++++++ .../reflect/ReflectionHelperTest.java | 39 +++++ 4 files changed, 328 insertions(+), 111 deletions(-) create mode 100644 gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java create mode 100644 gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java 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 d95087691e..6f0c8c1060 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 @@ -40,6 +40,7 @@ import java.io.IOException; import java.lang.reflect.Constructor; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.Type; import java.lang.reflect.Array; @@ -106,12 +107,6 @@ public TypeAdapter create(Gson gson, final TypeToken type) { return null; // it's a primitive! } - if (ReflectionHelper.isRecord(raw)) { - // We can assume blockInaccessible here, because it will never kick inn. We are not doing - // direct field access, and are instead constructing records via their canonical constructor. - return new RecordAdapter<>(raw, getBoundFields(gson, type, raw, true)); - } - FilterResult filterResult = ReflectionAccessFilterHelper.getFilterResult(reflectionFilters, raw); if (filterResult == FilterResult.BLOCK_ALL) { @@ -122,8 +117,14 @@ public TypeAdapter create(Gson gson, final TypeToken type) { } boolean blockInaccessible = filterResult == FilterResult.BLOCK_INACCESSIBLE; + // 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)); + } + ObjectConstructor constructor = constructorConstructor.get(type); - return new FieldReflectionAdapter<>(constructor, getBoundFields(gson, type, raw, blockInaccessible)); + return new FieldReflectionAdapter<>(constructor, getBoundFields(gson, type, raw, blockInaccessible, false)); } private static void checkAccessible(Object object, Field field) { @@ -136,7 +137,7 @@ private static void checkAccessible(Object object, Field field) { } private ReflectiveTypeAdapterFactory.BoundField createBoundField( - final Gson context, final Field field, final String name, + 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()); @@ -152,16 +153,18 @@ private ReflectiveTypeAdapterFactory.BoundField createBoundField( @SuppressWarnings("unchecked") final TypeAdapter typeAdapter = (TypeAdapter) mapped; - return new ReflectiveTypeAdapterFactory.BoundField(name, serialize, deserialize) { - @Override void write(JsonWriter writer, Object value) - throws IOException, IllegalAccessException { + return new ReflectiveTypeAdapterFactory.BoundField(name, field.getName(), serialize, deserialize) { + @Override void write(JsonWriter writer, Object source) + throws IOException, ReflectiveOperationException { if (!serialized) return; - if (blockInaccessible) { - checkAccessible(value, field); + if (blockInaccessible && accessor == null) { + checkAccessible(source, field); } - Object fieldValue = field.get(value); - if (fieldValue == value) { + Object fieldValue = (accessor != null) + ? accessor.invoke(source) + : field.get(source); + if (fieldValue == source) { // avoid direct recursion return; } @@ -172,25 +175,29 @@ private ReflectiveTypeAdapterFactory.BoundField createBoundField( } @Override - Object read(JsonReader reader) throws IOException { - return typeAdapter.read(reader); + void readIntoArray(JsonReader reader, int index, Object[] target) throws IOException { + Object fieldValue = typeAdapter.read(reader); + if (fieldValue != null || !isPrimitive) { + target[index] = fieldValue; + } } @Override - void read(JsonReader reader, Object value) + void readIntoField(JsonReader reader, Object target) throws IOException, IllegalAccessException { Object fieldValue = typeAdapter.read(reader); if (fieldValue != null || !isPrimitive) { if (blockInaccessible) { - checkAccessible(value, field); + checkAccessible(target, field); } - field.set(value, fieldValue); + field.set(target, fieldValue); } } }; } - private Map getBoundFields(Gson context, TypeToken type, Class raw, boolean blockInaccessible) { + private Map getBoundFields(Gson context, TypeToken type, Class raw, + boolean blockInaccessible, boolean isRecord) { Map result = new LinkedHashMap<>(); if (raw.isInterface()) { return result; @@ -218,8 +225,17 @@ private Map getBoundFields(Gson context, TypeToken type, if (!serialize && !deserialize) { continue; } + // 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 + Method accessor = null; + if (isRecord) { + accessor = ReflectionHelper.getAccessor(raw, field); + } - // If blockInaccessible, skip and perform access check later + // 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) { ReflectionHelper.makeAccessible(field); } @@ -229,7 +245,7 @@ private Map getBoundFields(Gson context, TypeToken type, for (int i = 0, size = fieldNames.size(); i < size; ++i) { String name = fieldNames.get(i); if (i != 0) serialize = false; // only serialize the default name - BoundField boundField = createBoundField(context, field, name, + BoundField boundField = createBoundField(context, field, accessor, name, TypeToken.get(fieldType), serialize, deserialize, blockInaccessible); BoundField replaced = result.put(name, boundField); if (previous == null) previous = replaced; @@ -247,35 +263,42 @@ private Map getBoundFields(Gson context, TypeToken type, static abstract class BoundField { final String name; + /** Name of the underlying field */ + final String componentName; final boolean serialized; final boolean deserialized; - protected BoundField(String name, boolean serialized, boolean deserialized) { + protected BoundField(String name, String componentName, boolean serialized, boolean deserialized) { this.name = name; + this.componentName = componentName; this.serialized = serialized; this.deserialized = deserialized; } - abstract void write(JsonWriter writer, Object value) throws IOException, IllegalAccessException; + /** 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 Object read(JsonReader reader) throws IOException; + /** 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 read(JsonReader reader, Object value) throws IOException, IllegalAccessException; + /** 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; } /** * Base class for Adapters produced by this factory. * - *

The {@link RecordAdapter} is a special case to handle Java 17 and later records, for all - * other types we use the {@link FieldReflectionAdapter}. This class encapuslates the common logic - * for serialization and deserialization. During deserialization, we construct an intermediary - * ResultHolder R, which we append values to. After the object has been read in full, the {@link - * #finalize(Object)} method is used to convert the intermediary to an instance of T. + *

The {@link RecordAdapter} is a special case to handle records for JVMs that support it, for + * all other types we use the {@link FieldReflectionAdapter}. This class encapsulates the common + * logic for serialization and deserialization. During deserialization, we construct an + * accumulator A, which we use to accumulate values from the source Json. After the object has been read in + * full, the {@link #finalize(Object)} method is used to convert the accumulator to an instance + * of T. * * @param type of objects that this Adapter creates. - * @param intermediary type used to build the deserialization result. + * @param type of accumulator used to build the deserialization result. */ - abstract static class Adapter extends TypeAdapter { + abstract static class Adapter extends TypeAdapter { protected final Map boundFields; protected Adapter(Map boundFields) { @@ -296,6 +319,8 @@ public void write(JsonWriter out, T value) throws IOException { } } catch (IllegalAccessException e) { throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e); + } catch (ReflectiveOperationException e) { + throw ReflectionHelper.createExceptionForRecordReflectionException(e); } out.endObject(); } @@ -307,7 +332,7 @@ public T read(JsonReader in) throws IOException { return null; } - I intermediary = createIntermediary(); + A accumulator = createAccumulator(); try { in.beginObject(); @@ -317,7 +342,7 @@ public T read(JsonReader in) throws IOException { if (field == null || !field.deserialized) { in.skipValue(); } else { - readField(intermediary, in, field); + readField(accumulator, in, field); } } } catch (IllegalStateException e) { @@ -326,18 +351,19 @@ public T read(JsonReader in) throws IOException { throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e); } in.endObject(); - return finalize(intermediary); + return finalize(accumulator); } - // Create the Object that will be used to collect each field value - abstract I createIntermediary(); - // Read a single Bounded field into the intermediary. The JsonReader will be pointed at the - // start of the value - // for the BoundField to read from. - abstract void readField(I intermediary, JsonReader in, BoundField field) + /** Create the Object that will be used to collect each field value */ + abstract A createAccumulator(); + /** + * Read a single Bounded field 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) throws IllegalAccessException, IOException; - // Convert the intermediary to a final instance of T. - abstract T finalize(I intermediary); + /** Convert the accumulator to a final instance of T. */ + abstract T finalize(A accumulator); } private static final class FieldReflectionAdapter extends Adapter { @@ -349,19 +375,19 @@ private static final class FieldReflectionAdapter extends Adapter { } @Override - T createIntermediary() { + T createAccumulator() { return constructor.construct(); } @Override - void readField(T intermediary, JsonReader in, BoundField field) + void readField(T accumulator, JsonReader in, BoundField field) throws IllegalAccessException, IOException { - field.read(in, intermediary); + field.readIntoField(in, accumulator); } @Override - T finalize(T intermediary) { - return intermediary; + T finalize(T accumulator) { + return accumulator; } } @@ -371,34 +397,39 @@ private static final class RecordAdapter extends Adapter { // Array of arguments to the constructor, initialized with default values for primitives private final Object[] constructorArgsDefaults; // Map from field names to index into the constructors arguments. - private final Map fieldIndices = new HashMap<>(); + private final Map componentIndices = new HashMap<>(); RecordAdapter(Class raw, Map boundFields) { super(boundFields); this.constructor = ReflectionHelper.getCanonicalRecordConstructor(raw); - String[] recordFields = ReflectionHelper.recordFields(raw); + String[] recordFields = ReflectionHelper.getRecordComponentNames(raw); for (int i = 0; i < recordFields.length; i++) { - fieldIndices.put(recordFields[i], i); + componentIndices.put(recordFields[i], i); } Class[] parameterTypes = constructor.getParameterTypes(); + + // We need to ensure that we are passing non-null values to primitive fields in the constructor. To do this, + // we create an Object[] where all primitives are initialized to non-null values. constructorArgsDefaults = new Object[parameterTypes.length]; for (int i = 0; i < parameterTypes.length; i++) { if (parameterTypes[i].isPrimitive()) { + // Voodoo magic, we create a new instance of this primitive type using reflection via an + // array. The array has 1 element, that of course will be initialized to the primitives + // default value. We then retrieve this value back from the array to get the properly + // initialized default value for the primitve type. constructorArgsDefaults[i] = Array.get(Array.newInstance(parameterTypes[i], 1), 0); } } } @Override - Object[] createIntermediary() { - Object[] constructorArgs = new Object[constructorArgsDefaults.length]; - System.arraycopy(constructorArgsDefaults, 0, constructorArgs, 0, constructorArgs.length); - return constructorArgs; + Object[] createAccumulator() { + return constructorArgsDefaults.clone(); } @Override - void readField(Object[] intermediary, JsonReader in, BoundField field) throws IOException { - Integer fieldIndex = fieldIndices.get(field.name); + void readField(Object[] accumulator, JsonReader in, BoundField field) throws IOException { + Integer fieldIndex = componentIndices.get(field.componentName); if (fieldIndex == null) { throw new IllegalStateException( "Could not find the index in the constructor " @@ -410,17 +441,17 @@ void readField(Object[] intermediary, JsonReader in, BoundField field) throws IO + " 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."); } - intermediary[fieldIndex] = field.read(in); + field.readIntoArray(in, fieldIndex, accumulator); } @Override @SuppressWarnings("unchecked") - T finalize(Object[] intermediary) { + T finalize(Object[] accumulator) { try { - return (T) constructor.newInstance(intermediary); + return (T) constructor.newInstance(accumulator); } catch (ReflectiveOperationException e) { throw new RuntimeException( - "Failed to invoke " + constructor + " with args " + Arrays.toString(intermediary), e); + "Failed to invoke " + constructor + " with args " + Arrays.toString(accumulator), e); } } } 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 f8f29fc70d..8c0070aaa6 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 @@ -5,7 +5,6 @@ import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.lang.reflect.Field; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; public class ReflectionHelper { @@ -82,12 +81,22 @@ public static String tryMakeAccessible(Constructor constructor) { } } + /** If records are supported on the JVM, this is equivalent to a call to Class.isRecord() */ public static boolean isRecord(Class raw) { return RECORD_HELPER.isRecord(raw); } - public static String[] recordFields(Class raw) { - return RECORD_HELPER.recordFields(raw); + public static String[] getRecordComponentNames(Class raw) { + return RECORD_HELPER.getRecordComponentNames(raw); + } + + /** Looks up the record accessor method that corresponds to the given record field */ + public static Method getAccessor(Class raw, Field field) { + return RECORD_HELPER.getAccessor(raw, field); + } + + public static Constructor getCanonicalRecordConstructor(Class raw) { + return RECORD_HELPER.getCanonicalRecordConstructor(raw); } public static RuntimeException createExceptionForUnexpectedIllegalAccess( @@ -98,8 +107,15 @@ public static RuntimeException createExceptionForUnexpectedIllegalAccess( exception); } - public static Constructor getCanonicalRecordConstructor(Class raw) { - return RECORD_HELPER.getCanonicalRecordConstructor(raw); + + public 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", + exception); } /** @@ -108,9 +124,11 @@ public static Constructor getCanonicalRecordConstructor(Class raw) { private abstract static class RecordHelper { abstract boolean isRecord(Class clazz); - abstract String[] recordFields(Class clazz); + abstract String[] getRecordComponentNames(Class clazz); abstract Constructor getCanonicalRecordConstructor(Class raw); + + public abstract Method getAccessor(Class raw, Field field); } private static class RecordSupportedHelper extends RecordHelper { @@ -118,71 +136,62 @@ private static class RecordSupportedHelper extends RecordHelper { private final Method getRecordComponents; private final Method getName; private final Method getType; + private final Method getAccessor; private RecordSupportedHelper() throws NoSuchMethodException { isRecord = Class.class.getMethod("isRecord"); - getRecordComponents = Class.class.getDeclaredMethod("getRecordComponents"); + getRecordComponents = Class.class.getMethod("getRecordComponents"); Class recordComponentType = getRecordComponents.getReturnType().getComponentType(); - getName = recordComponentType.getDeclaredMethod("getName"); - getType = recordComponentType.getDeclaredMethod("getType"); + getName = recordComponentType.getMethod("getName"); + getType = recordComponentType.getMethod("getType"); + getAccessor = recordComponentType.getMethod("getAccessor"); } @Override boolean isRecord(Class raw) { try { - return boolean.class.cast(isRecord.invoke(raw)); - } catch (IllegalAccessException e) { - throw createExceptionForUnexpectedIllegalAccess(e); - } catch (InvocationTargetException e) { - throw new RuntimeException("Failed to reflect into record components on" - + " class [" + raw + "], this is unexpected behaviour, as these methods should" - + " not throw when they are defined on Class", - e - ); + return Boolean.class.cast(isRecord.invoke(raw)).booleanValue(); + } catch (ReflectiveOperationException e) { + throw createExceptionForRecordReflectionException(e); } } @Override - String[] recordFields(Class raw) { + String[] getRecordComponentNames(Class raw) { try { - Object recordComponents = getRecordComponents.invoke(raw); - int componentCount = Array.getLength(recordComponents); - String[] recordFields = new String[componentCount]; - for (int i = 0; i < componentCount; i++) { - recordFields[i] = - String.class.cast(getName.invoke(Array.get(recordComponents, i))); + Object[] recordComponents = (Object[]) getRecordComponents.invoke(raw); + String[] componentNames = new String[recordComponents.length]; + for (int i = 0; i < recordComponents.length; i++) { + componentNames[i] = (String) getName.invoke(recordComponents[i]); } - return recordFields; - } catch (IllegalAccessException e) { - throw createExceptionForUnexpectedIllegalAccess(e); + return componentNames; } catch (ReflectiveOperationException e) { - throw new RuntimeException("Failed to reflect into record components on" - + " class [" + raw + "], this is unexpected behaviour, as these methods should" - + " not throw when they are defined on Class", - e - ); + throw createExceptionForRecordReflectionException(e); } } @Override public Constructor getCanonicalRecordConstructor(Class raw) { try { - Object recordComponents = getRecordComponents.invoke(raw); - int componentCount = Array.getLength(recordComponents); - Class[] recordFields = new Class[componentCount]; - for (int i = 0; i < componentCount; i++) { - recordFields[i] = - Class.class.cast(getType.invoke(Array.get(recordComponents, i))); + Object[] recordComponents = (Object[]) getRecordComponents.invoke(raw); + Class[] recordComponentTypes = new Class[recordComponents.length]; + for (int i = 0; i < recordComponents.length; i++) { + recordComponentTypes[i] = (Class) getType.invoke(recordComponents[i]); } - return raw.getConstructor(recordFields); - } catch (IllegalAccessException e) { - throw createExceptionForUnexpectedIllegalAccess(e); + return raw.getDeclaredConstructor(recordComponentTypes); } catch (ReflectiveOperationException e) { - throw new RuntimeException("Failed to reflect into record components on" - + " class [" + raw + "], this is unexpected behaviour, as these methods should" - + " not throw when they are defined on Class", - e - ); + throw createExceptionForRecordReflectionException(e); + } + } + + @Override + public Method getAccessor(Class raw, Field field) { + try { + // Records consists of record components, each with a unique name, a corresponding field and accessor method + // with the same name. Ref.: https://docs.oracle.com/javase/specs/jls/se17/html/jls-8.html#jls-8.10.3 + return raw.getMethod(field.getName()); + } catch (ReflectiveOperationException e) { + throw createExceptionForRecordReflectionException(e); } } } @@ -198,7 +207,7 @@ boolean isRecord(Class clazz) { } @Override - String[] recordFields(Class clazz) { + String[] getRecordComponentNames(Class clazz) { throw new UnsupportedOperationException( "Records are not supported on this JVM, this method should not be called"); } @@ -208,5 +217,11 @@ Constructor getCanonicalRecordConstructor(Class raw) { throw new UnsupportedOperationException( "Records are not supported on this JVM, this method should not be called"); } + + @Override + public Method getAccessor(Class raw, Field field) { + throw new UnsupportedOperationException( + "Records are not supported on this JVM, this method should not be called"); + } } } diff --git a/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java b/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java new file mode 100644 index 0000000000..840f8d3266 --- /dev/null +++ b/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java @@ -0,0 +1,132 @@ +package com.google.gson.internal.bind; + +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; +import com.google.gson.TypeAdapter; +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonWriter; +import org.junit.AssumptionViolatedException; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.UserPrincipal; +import java.util.Objects; + +import static org.junit.Assert.*; + +public class ReflectiveTypeAdapterFactoryTest { + + + @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"); + } + } + + @Test + public void testCustomAdapterForRecords() throws ClassNotFoundException { + Class unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); + + Gson gson = new Gson(); + TypeAdapter recordAdapter = gson.getAdapter(unixDomainPrincipalClass); + TypeAdapter defaultReflectionAdapter = gson.getAdapter(GroupPrincipalImpl.class); + assertNotEquals(recordAdapter.getClass(), defaultReflectionAdapter.getClass()); + } + + @Test + public void testSerializeRecords() throws ReflectiveOperationException { + Class unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); + Gson gson = new GsonBuilder() + .registerTypeAdapter(UserPrincipal.class, new TypeAdapter() { + @Override + public void write(JsonWriter out, UserPrincipal principal) throws IOException { + out.value(principal.getName()); + } + + @Override + public UserPrincipal read(JsonReader in) throws IOException { + final String name = in.nextString(); + return new UserPrincipalImpl(name); + } + }) + .registerTypeAdapter(GroupPrincipal.class, new TypeAdapter() { + @Override + public void write(JsonWriter out, GroupPrincipal value) throws IOException { + out.value(value.getName()); + } + + @Override + public GroupPrincipal read(JsonReader in) throws IOException { + final String name = in.nextString(); + return new GroupPrincipalImpl(name); + } + }) + .create(); + UserPrincipal userPrincipal = gson.fromJson("\"user\"", UserPrincipal.class); + GroupPrincipal groupPrincipal = gson.fromJson("\"group\"", GroupPrincipal.class); + Object recordInstance = unixDomainPrincipalClass.getDeclaredConstructor(UserPrincipal.class, GroupPrincipal.class) + .newInstance(userPrincipal, groupPrincipal); + String serialized = gson.toJson(recordInstance); + Object deserializedRecordInstance = gson.fromJson(serialized, unixDomainPrincipalClass); + + assertEquals(recordInstance, deserializedRecordInstance); + } + + private static class GroupPrincipalImpl implements GroupPrincipal { + private final String _name; + + public GroupPrincipalImpl(String name) { + _name = name; + } + + @Override + public String getName() { + return _name; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + GroupPrincipalImpl that = (GroupPrincipalImpl) o; + return Objects.equals(_name, that._name); + } + + @Override + public int hashCode() { + return Objects.hash(_name); + } + } + + private static class UserPrincipalImpl implements UserPrincipal { + private final String _name; + + public UserPrincipalImpl(String name) { + _name = name; + } + + @Override + public String getName() { + return _name; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + UserPrincipalImpl that = (UserPrincipalImpl) o; + return Objects.equals(_name, that._name); + } + + @Override + public int hashCode() { + return Objects.hash(_name); + } + } +} \ No newline at end of file diff --git a/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java b/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java new file mode 100644 index 0000000000..2c1b364740 --- /dev/null +++ b/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java @@ -0,0 +1,39 @@ +package com.google.gson.internal.reflect; + +import org.junit.*; + +import java.lang.reflect.Constructor; +import java.nio.file.attribute.GroupPrincipal; +import java.nio.file.attribute.UserPrincipal; + +import static org.junit.Assert.*; + +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"); + } + } + + @Test + public void testJava17Record() throws ClassNotFoundException { + Class unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); + // UnixDomainPrincipal is a record + assertTrue(ReflectionHelper.isRecord(unixDomainPrincipalClass)); + // with 2 fields + assertArrayEquals(new String[] {"user", "group"}, ReflectionHelper.getRecordComponentNames(unixDomainPrincipalClass)); + // Check canonical constructor + Constructor constructor = ReflectionHelper.getCanonicalRecordConstructor(unixDomainPrincipalClass); + assertNotNull(constructor); + assertArrayEquals( + new Class[] { UserPrincipal.class, GroupPrincipal.class }, + constructor.getParameterTypes()); + } + + +} \ No newline at end of file From c4c1730a72fe39db0e5d9ea6aca253fe5ef0e7da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=CC=8Ale=20Undheim?= Date: Sun, 9 Oct 2022 21:24:09 +0200 Subject: [PATCH 3/4] Fixed additional comments from @Marcono1234 --- .../bind/ReflectiveTypeAdapterFactory.java | 29 +-- .../internal/reflect/ReflectionHelper.java | 42 +++- .../ReflectiveTypeAdapterFactoryTest.java | 181 +++++++----------- .../reflect/ReflectionHelperTest.java | 101 +++++++--- 4 files changed, 192 insertions(+), 161 deletions(-) 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 6f0c8c1060..06aa971a83 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 @@ -227,8 +227,10 @@ private Map getBoundFields(Gson context, TypeToken type, } // 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) { + if (isRecord && !Modifier.isStatic(field.getModifiers())) { accessor = ReflectionHelper.getAccessor(raw, field); } @@ -264,18 +266,18 @@ private Map getBoundFields(Gson context, TypeToken type, static abstract class BoundField { final String name; /** Name of the underlying field */ - final String componentName; + final String fieldName; final boolean serialized; final boolean deserialized; - protected BoundField(String name, String componentName, boolean serialized, boolean deserialized) { + protected BoundField(String name, String fieldName, boolean serialized, boolean deserialized) { this.name = name; - this.componentName = componentName; + this.fieldName = fieldName; this.serialized = serialized; this.deserialized = deserialized; } - /** Read this field value from the source, and append its json value to the writer */ + /** 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; /** Read the value into the target array, used to provide constructor arguments for records */ @@ -291,7 +293,7 @@ protected BoundField(String name, String componentName, boolean serialized, bool *

The {@link RecordAdapter} is a special case to handle records for JVMs that support it, for * all other types we use the {@link FieldReflectionAdapter}. This class encapsulates the common * logic for serialization and deserialization. During deserialization, we construct an - * accumulator A, which we use to accumulate values from the source Json. After the object has been read in + * accumulator A, which we use to accumulate values from the source JSON. After the object has been read in * full, the {@link #finalize(Object)} method is used to convert the accumulator to an instance * of T. * @@ -357,7 +359,7 @@ public T read(JsonReader in) throws IOException { /** Create the Object that will be used to collect each field value */ abstract A createAccumulator(); /** - * Read a single Bounded field into the accumulator. The JsonReader will be pointed at the + * Read a single BoundedField 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) @@ -396,15 +398,18 @@ private static final class RecordAdapter extends Adapter { private final Constructor constructor; // Array of arguments to the constructor, initialized with default values for primitives private final Object[] constructorArgsDefaults; - // Map from field names to index into the constructors arguments. + // Map from component names to index into the constructors arguments. private final Map componentIndices = new HashMap<>(); RecordAdapter(Class raw, Map boundFields) { super(boundFields); this.constructor = ReflectionHelper.getCanonicalRecordConstructor(raw); - String[] recordFields = ReflectionHelper.getRecordComponentNames(raw); - for (int i = 0; i < recordFields.length; i++) { - componentIndices.put(recordFields[i], i); + // Ensure the constructor is accessible + ReflectionHelper.makeAccessible(this.constructor); + + String[] componentNames = ReflectionHelper.getRecordComponentNames(raw); + for (int i = 0; i < componentNames.length; i++) { + componentIndices.put(componentNames[i], i); } Class[] parameterTypes = constructor.getParameterTypes(); @@ -429,7 +434,7 @@ Object[] createAccumulator() { @Override void readField(Object[] accumulator, JsonReader in, BoundField field) throws IOException { - Integer fieldIndex = componentIndices.get(field.componentName); + Integer fieldIndex = componentIndices.get(field.fieldName); if (fieldIndex == null) { throw new IllegalStateException( "Could not find the index in the constructor " 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 8c0070aaa6..f55b30f5ba 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 @@ -2,10 +2,8 @@ import com.google.gson.JsonIOException; import com.google.gson.internal.GsonBuildConfig; -import java.lang.reflect.Array; -import java.lang.reflect.Constructor; -import java.lang.reflect.Field; -import java.lang.reflect.Method; + +import java.lang.reflect.*; public class ReflectionHelper { @@ -32,12 +30,36 @@ private ReflectionHelper() {} * @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 { try { - field.setAccessible(true); + object.setAccessible(true); } catch (Exception exception) { - throw new JsonIOException("Failed making field '" + field.getDeclaringClass().getName() + "#" - + field.getName() + "' accessible; either change its visibility or write a custom " - + "TypeAdapter for its declaring type", exception); + throw new JsonIOException("Failed making " + description + "' accessible; either change its visibility " + + "or write a custom TypeAdapter for its declaring type", exception); } } @@ -114,7 +136,7 @@ public static RuntimeException createExceptionForRecordReflectionException( + "(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 exists in the JVM. This exception is unexpected behaviour.", exception); } @@ -178,6 +200,8 @@ public Constructor getCanonicalRecordConstructor(Class raw) { for (int i = 0; i < recordComponents.length; i++) { recordComponentTypes[i] = (Class) getType.invoke(recordComponents[i]); } + // Uses getDeclaredConstructor because implicit constructor has same visibility as record and might + // therefore not be public return raw.getDeclaredConstructor(recordComponentTypes); } catch (ReflectiveOperationException e) { throw createExceptionForRecordReflectionException(e); diff --git a/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java b/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java index 840f8d3266..08c92f805d 100644 --- a/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java +++ b/gson/src/test/java/com/google/gson/internal/bind/ReflectiveTypeAdapterFactoryTest.java @@ -1,132 +1,83 @@ package com.google.gson.internal.bind; +import static org.junit.Assert.*; + 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.stream.JsonReader; import com.google.gson.stream.JsonWriter; -import org.junit.AssumptionViolatedException; -import org.junit.Before; -import org.junit.Test; - import java.io.IOException; import java.nio.file.attribute.GroupPrincipal; import java.nio.file.attribute.UserPrincipal; -import java.util.Objects; - -import static org.junit.Assert.*; +import java.security.Principal; +import org.junit.AssumptionViolatedException; +import org.junit.Before; +import org.junit.Test; public class ReflectiveTypeAdapterFactoryTest { - - @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"); - } + // 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. + 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"); } - - @Test - public void testCustomAdapterForRecords() throws ClassNotFoundException { - Class unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); - - Gson gson = new Gson(); - TypeAdapter recordAdapter = gson.getAdapter(unixDomainPrincipalClass); - TypeAdapter defaultReflectionAdapter = gson.getAdapter(GroupPrincipalImpl.class); - assertNotEquals(recordAdapter.getClass(), defaultReflectionAdapter.getClass()); + } + + @Test + public void testCustomAdapterForRecords() { + Gson gson = new Gson(); + TypeAdapter recordAdapter = gson.getAdapter(unixDomainPrincipalClass); + TypeAdapter defaultReflectionAdapter = gson.getAdapter(UserPrincipal.class); + assertNotEquals(recordAdapter.getClass(), defaultReflectionAdapter.getClass()); + } + + @Test + public void testSerializeRecords() throws ReflectiveOperationException { + Gson gson = + new GsonBuilder() + .registerTypeAdapter(UserPrincipal.class, new PrincipalTypeAdapter<>()) + .registerTypeAdapter(GroupPrincipal.class, new PrincipalTypeAdapter<>()) + .create(); + + UserPrincipal userPrincipal = gson.fromJson("\"user\"", UserPrincipal.class); + GroupPrincipal groupPrincipal = gson.fromJson("\"group\"", GroupPrincipal.class); + Object recordInstance = + unixDomainPrincipalClass + .getDeclaredConstructor(UserPrincipal.class, GroupPrincipal.class) + .newInstance(userPrincipal, groupPrincipal); + String serialized = gson.toJson(recordInstance); + Object deserializedRecordInstance = gson.fromJson(serialized, unixDomainPrincipalClass); + + assertEquals(recordInstance, deserializedRecordInstance); + assertEquals("{\"user\":\"user\",\"group\":\"group\"}", serialized); + } + + private static class PrincipalTypeAdapter extends TypeAdapter { + @Override + public void write(JsonWriter out, T principal) throws IOException { + out.value(principal.getName()); } - @Test - public void testSerializeRecords() throws ReflectiveOperationException { - Class unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); - Gson gson = new GsonBuilder() - .registerTypeAdapter(UserPrincipal.class, new TypeAdapter() { - @Override - public void write(JsonWriter out, UserPrincipal principal) throws IOException { - out.value(principal.getName()); - } - - @Override - public UserPrincipal read(JsonReader in) throws IOException { - final String name = in.nextString(); - return new UserPrincipalImpl(name); - } - }) - .registerTypeAdapter(GroupPrincipal.class, new TypeAdapter() { - @Override - public void write(JsonWriter out, GroupPrincipal value) throws IOException { - out.value(value.getName()); - } - - @Override - public GroupPrincipal read(JsonReader in) throws IOException { - final String name = in.nextString(); - return new GroupPrincipalImpl(name); - } - }) - .create(); - UserPrincipal userPrincipal = gson.fromJson("\"user\"", UserPrincipal.class); - GroupPrincipal groupPrincipal = gson.fromJson("\"group\"", GroupPrincipal.class); - Object recordInstance = unixDomainPrincipalClass.getDeclaredConstructor(UserPrincipal.class, GroupPrincipal.class) - .newInstance(userPrincipal, groupPrincipal); - String serialized = gson.toJson(recordInstance); - Object deserializedRecordInstance = gson.fromJson(serialized, unixDomainPrincipalClass); - - assertEquals(recordInstance, deserializedRecordInstance); - } - - private static class GroupPrincipalImpl implements GroupPrincipal { - private final String _name; - - public GroupPrincipalImpl(String name) { - _name = name; - } - - @Override - public String getName() { - return _name; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - GroupPrincipalImpl that = (GroupPrincipalImpl) o; - return Objects.equals(_name, that._name); - } - - @Override - public int hashCode() { - return Objects.hash(_name); - } - } - - private static class UserPrincipalImpl implements UserPrincipal { - private final String _name; - - public UserPrincipalImpl(String name) { - _name = name; - } - - @Override - public String getName() { - return _name; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - UserPrincipalImpl that = (UserPrincipalImpl) o; - return Objects.equals(_name, that._name); - } - - @Override - public int hashCode() { - return Objects.hash(_name); - } + @Override + public T read(JsonReader in) throws IOException { + 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); + return principal; } -} \ No newline at end of file + } +} diff --git a/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java b/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java index 2c1b364740..7d0c9833f2 100644 --- a/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java +++ b/gson/src/test/java/com/google/gson/internal/reflect/ReflectionHelperTest.java @@ -1,39 +1,90 @@ package com.google.gson.internal.reflect; -import org.junit.*; +import static org.junit.Assert.*; import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.nio.file.attribute.GroupPrincipal; import java.nio.file.attribute.UserPrincipal; - -import static org.junit.Assert.*; +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"); - } + @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"); + } + } + + @Test + public void testJava17Record() throws ClassNotFoundException { + Class unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); + // UnixDomainPrincipal is a record + assertTrue(ReflectionHelper.isRecord(unixDomainPrincipalClass)); + // with 2 components + assertArrayEquals( + new String[] {"user", "group"}, + ReflectionHelper.getRecordComponentNames(unixDomainPrincipalClass)); + // Check canonical constructor + Constructor constructor = + ReflectionHelper.getCanonicalRecordConstructor(unixDomainPrincipalClass); + assertNotNull(constructor); + assertArrayEquals( + new Class[] {UserPrincipal.class, GroupPrincipal.class}, + constructor.getParameterTypes()); + } + + @Test + public void testJava17RecordAccessors() throws ReflectiveOperationException { + // Create an instance of UnixDomainPrincipal, using our custom implementation of UserPrincipal, + // and GroupPrincipal. Then attempt to access each component of the record using our accessor + // methods. + Class unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); + Object unixDomainPrincipal = + ReflectionHelper.getCanonicalRecordConstructor(unixDomainPrincipalClass) + .newInstance(new PrincipalImpl("user"), new PrincipalImpl("group")); + for (String componentName : + ReflectionHelper.getRecordComponentNames(unixDomainPrincipalClass)) { + Field componentField = unixDomainPrincipalClass.getDeclaredField(componentName); + Method accessor = ReflectionHelper.getAccessor(unixDomainPrincipalClass, componentField); + Object principal = accessor.invoke(unixDomainPrincipal); + + assertEquals(new PrincipalImpl(componentName), principal); + } + } + + /** Implementation of {@link UserPrincipal} and {@link GroupPrincipal} just for record tests. */ + public static class PrincipalImpl implements UserPrincipal, GroupPrincipal { + private final String name; + + public PrincipalImpl(String name) { + this.name = name; } - @Test - public void testJava17Record() throws ClassNotFoundException { - Class unixDomainPrincipalClass = Class.forName("jdk.net.UnixDomainPrincipal"); - // UnixDomainPrincipal is a record - assertTrue(ReflectionHelper.isRecord(unixDomainPrincipalClass)); - // with 2 fields - assertArrayEquals(new String[] {"user", "group"}, ReflectionHelper.getRecordComponentNames(unixDomainPrincipalClass)); - // Check canonical constructor - Constructor constructor = ReflectionHelper.getCanonicalRecordConstructor(unixDomainPrincipalClass); - assertNotNull(constructor); - assertArrayEquals( - new Class[] { UserPrincipal.class, GroupPrincipal.class }, - constructor.getParameterTypes()); + @Override + public String getName() { + return name; } + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + PrincipalImpl principal = (PrincipalImpl) o; + return Objects.equals(name, principal.name); + } -} \ No newline at end of file + @Override + public int hashCode() { + return Objects.hash(name); + } + } +} From b6b448b93bbaed28625359e4e78f7e4b88cf3c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sta=CC=8Ale=20Undheim?= Date: Tue, 11 Oct 2022 07:30:43 +0200 Subject: [PATCH 4/4] Made Adapter in ReflectiveTypeAdapterFactory public Fix comment from @eamonnmcmanus --- .../google/gson/internal/bind/ReflectiveTypeAdapterFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 06aa971a83..5caeb107f0 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 @@ -300,7 +300,7 @@ protected BoundField(String name, String fieldName, boolean serialized, boolean * @param type of objects that this Adapter creates. * @param type of accumulator used to build the deserialization result. */ - abstract static class Adapter extends TypeAdapter { + public static abstract class Adapter extends TypeAdapter { protected final Map boundFields; protected Adapter(Map boundFields) {