From 9eebd65cbac5071191fda1dc929a0b76393a2b35 Mon Sep 17 00:00:00 2001 From: Maciej Szwaja Date: Thu, 4 Jul 2024 15:22:56 +0000 Subject: [PATCH] make use of generic type info in *Utils classes --- .../sdk/schemas/utils/AutoValueUtils.java | 9 +- .../sdk/schemas/utils/ByteBuddyUtils.java | 23 ++- .../beam/sdk/schemas/utils/JavaBeanUtils.java | 12 +- .../beam/sdk/schemas/utils/POJOUtils.java | 138 ++++-------------- .../beam/sdk/schemas/utils/POJOUtilsTest.java | 80 ---------- 5 files changed, 59 insertions(+), 203 deletions(-) diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AutoValueUtils.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AutoValueUtils.java index 7bff2450b853..66e14737e9f6 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AutoValueUtils.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AutoValueUtils.java @@ -347,11 +347,10 @@ public ByteCodeAppender appender(final Target implementationTarget) { TypeConversion convertType = typeConversionsFactory.createTypeConversion(true); for (int i = 0; i < setters.size(); ++i) { - Method setterMethod = checkNotNull(setters.get(i).getMethod()); - Parameter parameter = setterMethod.getParameters()[0]; + FieldValueTypeInformation setterType = setters.get(i); + Method setterMethod = checkNotNull(setterType.getMethod()); ForLoadedType convertedType = - new ForLoadedType( - (Class) convertType.convert(TypeDescriptor.of(parameter.getParameterizedType()))); + new ForLoadedType((Class) convertType.convert(setterType.getType())); StackManipulation readParameter = new StackManipulation.Compound( @@ -366,7 +365,7 @@ public ByteCodeAppender appender(final Target implementationTarget) { Duplication.SINGLE, typeConversionsFactory .createSetterConversions(readParameter) - .convert(TypeDescriptor.of(parameter.getType())), + .convert(setterType.getType()), MethodInvocation.invoke(new ForLoadedMethod(setterMethod)), Removal.SINGLE); } diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java index e99459ddc60a..eb0fa7dea17f 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java @@ -426,12 +426,20 @@ protected Type convertDefault(TypeDescriptor type) { return returnRawTypes ? type.getRawType() : type.getType(); } + public static TypeDescriptor primitiveToWrapper(TypeDescriptor typeDescriptor) { + Class cls = typeDescriptor.getRawType(); + if (cls.isPrimitive()) { + return TypeDescriptor.of(ClassUtils.primitiveToWrapper(cls)); + } else { + return typeDescriptor; + } + } + @SuppressWarnings("unchecked") private TypeDescriptor> createCollectionType( TypeDescriptor componentType) { TypeDescriptor wrappedComponentType = - (TypeDescriptor) - TypeDescriptor.of(ClassUtils.primitiveToWrapper(componentType.getRawType())); + (TypeDescriptor) primitiveToWrapper(componentType); return new TypeDescriptor>() {}.where( new TypeParameter() {}, wrappedComponentType); } @@ -440,8 +448,7 @@ private TypeDescriptor> createCollectionType( private TypeDescriptor> createIterableType( TypeDescriptor componentType) { TypeDescriptor wrappedComponentType = - (TypeDescriptor) - TypeDescriptor.of(ClassUtils.primitiveToWrapper(componentType.getRawType())); + (TypeDescriptor) primitiveToWrapper(componentType); return new TypeDescriptor>() {}.where( new TypeParameter() {}, wrappedComponentType); } @@ -1510,10 +1517,10 @@ public ByteCodeAppender appender(final Target implementationTarget) { // Push all creator parameters on the stack. TypeConversion convertType = typeConversionsFactory.createTypeConversion(true); for (int i = 0; i < parameters.size(); i++) { - Parameter parameter = parameters.get(i); + FieldValueTypeInformation fieldType = + fields.get(Preconditions.checkNotNull(fieldMapping.get(i))); ForLoadedType convertedType = - new ForLoadedType( - (Class) convertType.convert(TypeDescriptor.of(parameter.getType()))); + new ForLoadedType((Class) convertType.convert(fieldType.getType())); // The instruction to read the parameter. Use the fieldMapping to reorder parameters as // necessary. @@ -1528,7 +1535,7 @@ public ByteCodeAppender appender(final Target implementationTarget) { stackManipulation, typeConversionsFactory .createSetterConversions(readParameter) - .convert(TypeDescriptor.of(parameter.getParameterizedType()))); + .convert(fieldType.getType())); } stackManipulation = new StackManipulation.Compound( diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JavaBeanUtils.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JavaBeanUtils.java index 32b4ef97b70e..41592c637701 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JavaBeanUtils.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/JavaBeanUtils.java @@ -31,6 +31,7 @@ import net.bytebuddy.ByteBuddy; import net.bytebuddy.asm.AsmVisitorWrapper; import net.bytebuddy.description.method.MethodDescription.ForLoadedMethod; +import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.scaffold.InstrumentedType; import net.bytebuddy.implementation.FixedValue; @@ -39,6 +40,7 @@ import net.bytebuddy.implementation.bytecode.ByteCodeAppender.Size; import net.bytebuddy.implementation.bytecode.Removal; import net.bytebuddy.implementation.bytecode.StackManipulation; +import net.bytebuddy.implementation.bytecode.assign.TypeCasting; import net.bytebuddy.implementation.bytecode.member.MethodInvocation; import net.bytebuddy.implementation.bytecode.member.MethodReturn; import net.bytebuddy.implementation.bytecode.member.MethodVariableAccess; @@ -439,6 +441,13 @@ public ByteCodeAppender appender(final Target implementationTarget) { return (methodVisitor, implementationContext, instrumentedMethod) -> { // this + method parameters. int numLocals = 1 + instrumentedMethod.getParameters().size(); + StackManipulation cast = + typeInformation + .getRawType() + .isAssignableFrom( + Preconditions.checkNotNull(typeInformation.getMethod()).getReturnType()) + ? StackManipulation.Trivial.INSTANCE + : TypeCasting.to(TypeDescription.ForLoadedType.of(typeInformation.getRawType())); // StackManipulation that will read the value from the class field. StackManipulation readValue = @@ -449,7 +458,8 @@ public ByteCodeAppender appender(final Target implementationTarget) { MethodInvocation.invoke( new ForLoadedMethod( Preconditions.checkNotNull( - typeInformation.getMethod(), GETTER_WITH_NULL_METHOD_ERROR)))); + typeInformation.getMethod(), GETTER_WITH_NULL_METHOD_ERROR))), + cast); StackManipulation stackManipulation = new StackManipulation.Compound( diff --git a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/POJOUtils.java b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/POJOUtils.java index 8e33d321a1c6..3aac12a9169b 100644 --- a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/POJOUtils.java +++ b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/POJOUtils.java @@ -30,6 +30,7 @@ import net.bytebuddy.ByteBuddy; import net.bytebuddy.asm.AsmVisitorWrapper; import net.bytebuddy.description.field.FieldDescription.ForLoadedField; +import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.description.type.TypeDescription.ForLoadedType; import net.bytebuddy.dynamic.DynamicType; import net.bytebuddy.dynamic.scaffold.InstrumentedType; @@ -151,18 +152,13 @@ private static SchemaUserTypeCreator createSetFieldCreator( Schema schema, List types, TypeConversionsFactory typeConversionsFactory) { - // Get the list of class fields ordered by schema. - List fields = - types.stream() - .map(type -> Preconditions.checkNotNull(type.getField())) - .collect(Collectors.toList()); try { DynamicType.Builder builder = BYTE_BUDDY .with(new InjectPackageStrategy(clazz)) .subclass(SchemaUserTypeCreator.class) .method(ElementMatchers.named("create")) - .intercept(new SetFieldCreateInstruction(fields, clazz, typeConversionsFactory)); + .intercept(new SetFieldCreateInstruction(types, clazz, typeConversionsFactory)); return builder .visit(new AsmVisitorWrapper.ForDeclaredMethods().writerFlags(ClassWriter.COMPUTE_FRAMES)) @@ -305,11 +301,8 @@ public static SchemaUserTypeCreator createStaticCreator( ByteBuddyUtils.subclassGetterInterface( BYTE_BUDDY, field.getDeclaringClass(), - typeConversionsFactory - .createTypeConversion(false) - .convert(TypeDescriptor.of(field.getType()))); - builder = - implementGetterMethods(builder, field, typeInformation.getName(), typeConversionsFactory); + typeConversionsFactory.createTypeConversion(false).convert(typeInformation.getType())); + builder = implementGetterMethods(builder, typeInformation, typeConversionsFactory); try { return builder .visit(new AsmVisitorWrapper.ForDeclaredMethods().writerFlags(ClassWriter.COMPUTE_FRAMES)) @@ -331,107 +324,25 @@ public static SchemaUserTypeCreator createStaticCreator( private static DynamicType.Builder> implementGetterMethods( DynamicType.Builder> builder, - Field field, - String name, + FieldValueTypeInformation typeInformation, TypeConversionsFactory typeConversionsFactory) { return builder .visit(new AsmVisitorWrapper.ForDeclaredMethods().writerFlags(ClassWriter.COMPUTE_FRAMES)) .method(ElementMatchers.named("name")) - .intercept(FixedValue.reference(name)) + .intercept(FixedValue.reference(typeInformation.getName())) .method(ElementMatchers.named("get")) - .intercept(new ReadFieldInstruction(field, typeConversionsFactory)); - } - - // The list of setters for a class is cached, so we only create the classes the first time - // getSetters is called. - private static final Map, List>> - CACHED_SETTERS = Maps.newConcurrentMap(); - - public static List> getSetters( - TypeDescriptor typeDescriptor, - Schema schema, - FieldValueTypeSupplier fieldValueTypeSupplier, - TypeConversionsFactory typeConversionsFactory) { - // Return the setters, ordered by their position in the schema. - return (List) - CACHED_SETTERS.computeIfAbsent( - TypeDescriptorWithSchema.create(typeDescriptor, schema), - c -> { - List types = - fieldValueTypeSupplier.get(typeDescriptor, schema); - return types.stream() - .map(t -> createSetter(t, typeConversionsFactory)) - .collect(Collectors.toList()); - }); - } - - /** - * Generate the following {@link FieldValueSetter} class for the {@link Field}. - * - * 

-   *   class Setter implements {@literal FieldValueSetter} {
-   *     {@literal @}Override public String name() { return field.getName(); }
-   *     {@literal @}Override public Class type() { return field.getType(); }
-   *     {@literal @}Override public Type elementType() { return elementType; }
-   *     {@literal @}Override public Type mapKeyType() { return mapKeyType; }
-   *     {@literal @}Override public Type mapValueType() { return mapValueType; }
-   *     {@literal @}Override public void set(POJO pojo, FieldType value) {
-   *        pojo.field = convert(value);
-   *      }
-   *   }
-   *
- */ - @SuppressWarnings("unchecked") - private static FieldValueSetter createSetter( - FieldValueTypeInformation typeInformation, TypeConversionsFactory typeConversionsFactory) { - Field field = Preconditions.checkNotNull(typeInformation.getField()); - DynamicType.Builder> builder = - ByteBuddyUtils.subclassSetterInterface( - BYTE_BUDDY, - field.getDeclaringClass(), - typeConversionsFactory - .createTypeConversion(false) - .convert(TypeDescriptor.of(field.getType()))); - builder = implementSetterMethods(builder, field, typeConversionsFactory); - try { - return builder - .visit(new AsmVisitorWrapper.ForDeclaredMethods().writerFlags(ClassWriter.COMPUTE_FRAMES)) - .make() - .load( - ReflectHelpers.findClassLoader(field.getDeclaringClass().getClassLoader()), - getClassLoadingStrategy(field.getDeclaringClass())) - .getLoaded() - .getDeclaredConstructor() - .newInstance(); - } catch (InstantiationException - | IllegalAccessException - | NoSuchMethodException - | InvocationTargetException e) { - throw new RuntimeException("Unable to generate a getter for field '" + field + "'.", e); - } - } - - private static - DynamicType.Builder> implementSetterMethods( - DynamicType.Builder> builder, - Field field, - TypeConversionsFactory typeConversionsFactory) { - return builder - .visit(new AsmVisitorWrapper.ForDeclaredMethods().writerFlags(ClassWriter.COMPUTE_FRAMES)) - .method(ElementMatchers.named("name")) - .intercept(FixedValue.reference(field.getName())) - .method(ElementMatchers.named("set")) - .intercept(new SetFieldInstruction(field, typeConversionsFactory)); + .intercept(new ReadFieldInstruction(typeInformation, typeConversionsFactory)); } // Implements a method to read a public field out of an object. static class ReadFieldInstruction implements Implementation { // Field that will be read. - private final Field field; + private final FieldValueTypeInformation typeInformation; private final TypeConversionsFactory typeConversionsFactory; - ReadFieldInstruction(Field field, TypeConversionsFactory typeConversionsFactory) { - this.field = field; + ReadFieldInstruction( + FieldValueTypeInformation typeInformation, TypeConversionsFactory typeConversionsFactory) { + this.typeInformation = typeInformation; this.typeConversionsFactory = typeConversionsFactory; } @@ -446,19 +357,25 @@ public ByteCodeAppender appender(final Target implementationTarget) { // this + method parameters. int numLocals = 1 + instrumentedMethod.getParameters().size(); + StackManipulation cast = + typeInformation.getRawType().isAssignableFrom(typeInformation.getField().getType()) + ? StackManipulation.Trivial.INSTANCE + : TypeCasting.to(TypeDescription.ForLoadedType.of(typeInformation.getRawType())); + // StackManipulation that will read the value from the class field. StackManipulation readValue = new StackManipulation.Compound( // Method param is offset 1 (offset 0 is the this parameter). MethodVariableAccess.REFERENCE.loadFrom(1), // Read the field from the object. - FieldAccess.forField(new ForLoadedField(field)).read()); + FieldAccess.forField(new ForLoadedField(typeInformation.getField())).read(), + cast); StackManipulation stackManipulation = new StackManipulation.Compound( typeConversionsFactory .createGetterConversions(readValue) - .convert(TypeDescriptor.of(field.getGenericType())), + .convert(typeInformation.getType()), MethodReturn.REFERENCE); StackManipulation.Size size = stackManipulation.apply(methodVisitor, implementationContext); @@ -513,13 +430,15 @@ public ByteCodeAppender appender(final Target implementationTarget) { // Implements a method to construct an object. static class SetFieldCreateInstruction implements Implementation { - private final List fields; + private final List typeInformations; private final Class pojoClass; private final TypeConversionsFactory typeConversionsFactory; SetFieldCreateInstruction( - List fields, Class pojoClass, TypeConversionsFactory typeConversionsFactory) { - this.fields = fields; + List typeInformations, + Class pojoClass, + TypeConversionsFactory typeConversionsFactory) { + this.typeInformations = typeInformations; this.pojoClass = pojoClass; this.typeConversionsFactory = typeConversionsFactory; } @@ -551,11 +470,12 @@ public ByteCodeAppender appender(final Target implementationTarget) { // The types in the POJO might be the types returned by Beam's Row class, // so we have to convert the types used by Beam's Row class. TypeConversion convertType = typeConversionsFactory.createTypeConversion(true); - for (int i = 0; i < fields.size(); ++i) { - Field field = fields.get(i); + for (int i = 0; i < typeInformations.size(); ++i) { + FieldValueTypeInformation typeInformation = typeInformations.get(i); + Field field = typeInformation.getField(); ForLoadedType convertedType = - new ForLoadedType((Class) convertType.convert(TypeDescriptor.of(field.getType()))); + new ForLoadedType((Class) convertType.convert(typeInformation.getType())); // The instruction to read the parameter. StackManipulation readParameter = @@ -572,7 +492,7 @@ public ByteCodeAppender appender(final Target implementationTarget) { // Do any conversions necessary. typeConversionsFactory .createSetterConversions(readParameter) - .convert(TypeDescriptor.of(field.getType())), + .convert(typeInformation.getType()), // Now update the field. FieldAccess.forField(new ForLoadedField(field)).write()); stackManipulation = new StackManipulation.Compound(stackManipulation, updateField); diff --git a/sdks/java/core/src/test/java/org/apache/beam/sdk/schemas/utils/POJOUtilsTest.java b/sdks/java/core/src/test/java/org/apache/beam/sdk/schemas/utils/POJOUtilsTest.java index 378cdc06805f..6b9fbcd30a27 100644 --- a/sdks/java/core/src/test/java/org/apache/beam/sdk/schemas/utils/POJOUtilsTest.java +++ b/sdks/java/core/src/test/java/org/apache/beam/sdk/schemas/utils/POJOUtilsTest.java @@ -23,7 +23,6 @@ import static org.apache.beam.sdk.schemas.utils.TestPOJOs.NESTED_POJO_SCHEMA; import static org.apache.beam.sdk.schemas.utils.TestPOJOs.NESTED_POJO_WITH_SIMPLE_POJO_SCHEMA; import static org.apache.beam.sdk.schemas.utils.TestPOJOs.POJO_WITH_BOXED_FIELDS_SCHEMA; -import static org.apache.beam.sdk.schemas.utils.TestPOJOs.POJO_WITH_BYTE_ARRAY_SCHEMA; import static org.apache.beam.sdk.schemas.utils.TestPOJOs.PRIMITIVE_ARRAY_POJO_SCHEMA; import static org.apache.beam.sdk.schemas.utils.TestPOJOs.PRIMITIVE_MAP_POJO_SCHEMA; import static org.apache.beam.sdk.schemas.utils.TestPOJOs.SIMPLE_POJO_SCHEMA; @@ -37,7 +36,6 @@ import java.nio.charset.StandardCharsets; import java.util.List; import org.apache.beam.sdk.schemas.FieldValueGetter; -import org.apache.beam.sdk.schemas.FieldValueSetter; import org.apache.beam.sdk.schemas.JavaFieldSchema.JavaFieldTypeSupplier; import org.apache.beam.sdk.schemas.Schema; import org.apache.beam.sdk.schemas.utils.ByteBuddyUtils.DefaultTypeConversionsFactory; @@ -46,7 +44,6 @@ import org.apache.beam.sdk.schemas.utils.TestPOJOs.NestedMapPOJO; import org.apache.beam.sdk.schemas.utils.TestPOJOs.NestedPOJO; import org.apache.beam.sdk.schemas.utils.TestPOJOs.POJOWithBoxedFields; -import org.apache.beam.sdk.schemas.utils.TestPOJOs.POJOWithByteArray; import org.apache.beam.sdk.schemas.utils.TestPOJOs.POJOWithNullables; import org.apache.beam.sdk.schemas.utils.TestPOJOs.PrimitiveArrayPOJO; import org.apache.beam.sdk.schemas.utils.TestPOJOs.PrimitiveMapPOJO; @@ -182,44 +179,6 @@ public void testGeneratedSimpleGetters() { assertEquals("stringBuilder", getters.get(11).get(simplePojo)); } - @Test - public void testGeneratedSimpleSetters() { - SimplePOJO simplePojo = new SimplePOJO(); - List> setters = - POJOUtils.getSetters( - new TypeDescriptor() {}, - SIMPLE_POJO_SCHEMA, - JavaFieldTypeSupplier.INSTANCE, - new DefaultTypeConversionsFactory()); - assertEquals(12, setters.size()); - - setters.get(0).set(simplePojo, "field1"); - setters.get(1).set(simplePojo, (byte) 41); - setters.get(2).set(simplePojo, (short) 42); - setters.get(3).set(simplePojo, (int) 43); - setters.get(4).set(simplePojo, (long) 44); - setters.get(5).set(simplePojo, true); - setters.get(6).set(simplePojo, DATE.toInstant()); - setters.get(7).set(simplePojo, INSTANT); - setters.get(8).set(simplePojo, BYTE_ARRAY); - setters.get(9).set(simplePojo, BYTE_BUFFER.array()); - setters.get(10).set(simplePojo, new BigDecimal(42)); - setters.get(11).set(simplePojo, "stringBuilder"); - - assertEquals("field1", simplePojo.str); - assertEquals((byte) 41, simplePojo.aByte); - assertEquals((short) 42, simplePojo.aShort); - assertEquals((int) 43, simplePojo.anInt); - assertEquals((long) 44, simplePojo.aLong); - assertTrue(simplePojo.aBoolean); - assertEquals(DATE, simplePojo.dateTime); - assertEquals(INSTANT, simplePojo.instant); - assertArrayEquals("Unexpected bytes", BYTE_ARRAY, simplePojo.bytes); - assertEquals(BYTE_BUFFER, simplePojo.byteBuffer); - assertEquals(new BigDecimal(42), simplePojo.bigDecimal); - assertEquals("stringBuilder", simplePojo.stringBuilder.toString()); - } - @Test public void testGeneratedSimpleBoxedGetters() { POJOWithBoxedFields pojo = new POJOWithBoxedFields((byte) 41, (short) 42, 43, 44L, true); @@ -236,43 +195,4 @@ public void testGeneratedSimpleBoxedGetters() { assertEquals((long) 44, getters.get(3).get(pojo)); assertTrue((Boolean) getters.get(4).get(pojo)); } - - @Test - public void testGeneratedSimpleBoxedSetters() { - POJOWithBoxedFields pojo = new POJOWithBoxedFields(); - List> setters = - POJOUtils.getSetters( - new TypeDescriptor() {}, - POJO_WITH_BOXED_FIELDS_SCHEMA, - JavaFieldTypeSupplier.INSTANCE, - new DefaultTypeConversionsFactory()); - - setters.get(0).set(pojo, (byte) 41); - setters.get(1).set(pojo, (short) 42); - setters.get(2).set(pojo, (int) 43); - setters.get(3).set(pojo, (long) 44); - setters.get(4).set(pojo, true); - - assertEquals((byte) 41, pojo.aByte.byteValue()); - assertEquals((short) 42, pojo.aShort.shortValue()); - assertEquals((int) 43, pojo.anInt.intValue()); - assertEquals((long) 44, pojo.aLong.longValue()); - assertTrue(pojo.aBoolean.booleanValue()); - } - - @Test - public void testGeneratedByteBufferSetters() { - POJOWithByteArray pojo = new POJOWithByteArray(); - List> setters = - POJOUtils.getSetters( - new TypeDescriptor() {}, - POJO_WITH_BYTE_ARRAY_SCHEMA, - JavaFieldTypeSupplier.INSTANCE, - new DefaultTypeConversionsFactory()); - setters.get(0).set(pojo, BYTE_ARRAY); - setters.get(1).set(pojo, BYTE_BUFFER.array()); - - assertArrayEquals("not equal", BYTE_ARRAY, pojo.bytes1); - assertEquals(BYTE_BUFFER, pojo.bytes2); - } }