From f4ad8d3e70547c6ff6804355ec911bb09cbb309a Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Thu, 4 Jan 2018 15:36:42 +0000 Subject: [PATCH] CORDA-902 - AMQP Setter Construction when empty / no constructor --- .../amqp/CorDappCustomSerializer.kt | 2 +- .../serialization/amqp/CustomSerializer.kt | 2 +- .../serialization/amqp/EvolutionSerializer.kt | 2 +- .../serialization/amqp/ObjectSerializer.kt | 52 ++- .../internal/serialization/amqp/Schema.kt | 9 +- .../serialization/amqp/SerializationHelper.kt | 60 +++- .../serialization/amqp/SerializationOutput.kt | 16 + .../amqp/custom/ThrowableSerializer.kt | 2 +- .../amqp/SetterConstructorTests.java | 295 ++++++++++++++++++ .../serialization/amqp/AMQPTestUtils.kt | 17 - 10 files changed, 419 insertions(+), 38 deletions(-) create mode 100644 node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/SetterConstructorTests.java diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CorDappCustomSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CorDappCustomSerializer.kt index 72373488b3..8e5e249457 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CorDappCustomSerializer.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CorDappCustomSerializer.kt @@ -70,7 +70,7 @@ class CorDappCustomSerializer( data.withDescribed(descriptor) { data.withList { - for (property in proxySerializer.propertySerializers) { + for (property in proxySerializer.propertySerializers.getters) { property.writeProperty(proxy, this, output) } } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CustomSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CustomSerializer.kt index 06cef5b6cd..647da2fd05 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CustomSerializer.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/CustomSerializer.kt @@ -132,7 +132,7 @@ abstract class CustomSerializer : AMQPSerializer, SerializerFor { override fun writeDescribedObject(obj: T, data: Data, type: Type, output: SerializationOutput) { val proxy = toProxy(obj) data.withList { - for (property in proxySerializer.propertySerializers) { + for (property in proxySerializer.propertySerializers.getters) { property.writeProperty(proxy, this, output) } } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt index 797943cf93..663a153cbd 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt @@ -20,7 +20,7 @@ class EvolutionSerializer( override val kotlinConstructor: KFunction?) : ObjectSerializer(clazz, factory) { // explicitly set as empty to indicate it's unused by this type of serializer - override val propertySerializers: Collection = emptyList() + override val propertySerializers = ConstructorDestructorMethods (emptyList(), emptyList()) /** * Represents a parameter as would be passed to the constructor of the class as it was diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt index b405b99a98..bd171d9f00 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/ObjectSerializer.kt @@ -21,7 +21,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS private val logger = contextLogger() } - open internal val propertySerializers: Collection by lazy { + open internal val propertySerializers: ConstructorDestructorMethods by lazy { propertiesForSerialization(kotlinConstructor, clazz, factory) } @@ -37,7 +37,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS for (iface in interfaces) { output.requireSerializer(iface) } - for (property in propertySerializers) { + for (property in propertySerializers.getters) { property.writeClassInfo(output) } } @@ -48,7 +48,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS data.withDescribed(typeNotation.descriptor) { // Write list withList { - for (property in propertySerializers) { + for (property in propertySerializers.getters) { property.writeProperty(obj, this, output) } } @@ -60,22 +60,58 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS schemas: SerializationSchemas, input: DeserializationInput): Any = ifThrowsAppend({ clazz.typeName }) { if (obj is List<*>) { - if (obj.size > propertySerializers.size) { + if (obj.size > propertySerializers.getters.size) { throw NotSerializableException("Too many properties in described type $typeName") } - val params = obj.zip(propertySerializers).map { it.second.readProperty(it.first, schemas, input) } - construct(params) + + return if (propertySerializers.setters.isEmpty()) { + readObjectBuildViaConstructor(obj, schemas, input) + } else { + readObjectBuildViaSetters(obj, schemas, input) + } } else throw NotSerializableException("Body of described type is unexpected $obj") } + private fun readObjectBuildViaConstructor( + obj: List<*>, + schemas: SerializationSchemas, + input: DeserializationInput) : Any = ifThrowsAppend({ clazz.typeName }){ + logger.debug { "Calling construction based construction" } + + return construct(obj.zip(propertySerializers.getters).map { it.second.readProperty(it.first, schemas, input) }) + } + + private fun readObjectBuildViaSetters( + obj: List<*>, + schemas: SerializationSchemas, + input: DeserializationInput) : Any = ifThrowsAppend({ clazz.typeName }){ + logger.debug { "Calling setter based construction" } + + val instance : Any = javaConstructor?.newInstance() ?: throw NotSerializableException ( + "Failed to instantiate instance of object $clazz") + + // read the properties out of the serialised form + val propertiesFromBlob = obj + .zip(propertySerializers.getters) + .map { it.second.readProperty(it.first, schemas, input) } + + // one by one take a property and invoke the setter on the class + propertySerializers.setters.zip(propertiesFromBlob).forEach { + it.first?.invoke(instance, *listOf(it.second).toTypedArray()) + } + + return instance + } + private fun generateFields(): List { - return propertySerializers.map { Field(it.name, it.type, it.requires, it.default, null, it.mandatory, false) } + return propertySerializers.getters.map { + Field(it.name, it.type, it.requires, it.default, null, it.mandatory, false) + } } private fun generateProvides(): List = interfaces.map { nameForType(it) } fun construct(properties: List): Any { - logger.debug { "Calling constructor: '$javaConstructor' with properties '$properties'" } return javaConstructor?.newInstance(*properties.toTypedArray()) ?: diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt index bb38933ed0..79b2a57b93 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt @@ -419,9 +419,12 @@ private fun isCollectionOrMap(type: Class<*>) = (Collection::class.java.isAssign private fun fingerprintForObject(type: Type, contextType: Type?, alreadySeen: MutableSet, hasher: Hasher, factory: SerializerFactory): Hasher { // Hash the class + properties + interfaces val name = type.asClass()?.name ?: throw NotSerializableException("Expected only Class or ParameterizedType but found $type") - propertiesForSerialization(constructorForDeserialization(type), contextType ?: type, factory).fold(hasher.putUnencodedChars(name)) { orig, prop -> - fingerprintForType(prop.resolvedType, type, alreadySeen, orig, factory).putUnencodedChars(prop.name).putUnencodedChars(if (prop.mandatory) NOT_NULLABLE_HASH else NULLABLE_HASH) - } + propertiesForSerialization(constructorForDeserialization(type), contextType ?: type, factory).getters + .fold(hasher.putUnencodedChars(name)) { orig, prop -> + fingerprintForType(prop.resolvedType, type, alreadySeen, orig, factory) + .putUnencodedChars(prop.name) + .putUnencodedChars(if (prop.mandatory) NOT_NULLABLE_HASH else NULLABLE_HASH) + } interfacesForSerialization(type, factory).map { fingerprintForType(it, type, alreadySeen, hasher, factory) } return hasher } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt index b3f6d5ce53..a90bf7a706 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt @@ -2,12 +2,14 @@ package net.corda.nodeapi.internal.serialization.amqp import com.google.common.primitives.Primitives import com.google.common.reflect.TypeToken +import io.netty.util.internal.EmptyArrays import net.corda.core.serialization.ClassWhitelist import net.corda.core.serialization.CordaSerializable import net.corda.core.serialization.SerializationContext import org.apache.qpid.proton.codec.Data import java.beans.IndexedPropertyDescriptor import java.beans.Introspector +import java.beans.PropertyDescriptor import java.io.NotSerializableException import java.lang.reflect.* import java.util.* @@ -26,6 +28,10 @@ import kotlin.reflect.jvm.javaType @Retention(AnnotationRetention.RUNTIME) annotation class ConstructorForDeserialization +data class ConstructorDestructorMethods( + val getters : Collection, + val setters : Collection) + /** * Code for finding the constructor we will use for deserialization. * @@ -67,18 +73,30 @@ internal fun constructorForDeserialization(type: Type): KFunction? { * Note, you will need any Java classes to be compiled with the `-parameters` option to ensure constructor parameters have * names accessible via reflection. */ -internal fun propertiesForSerialization(kotlinConstructor: KFunction?, type: Type, factory: SerializerFactory): Collection { +internal fun propertiesForSerialization(kotlinConstructor: KFunction?, type: Type, factory: SerializerFactory): ConstructorDestructorMethods { val clazz = type.asClass()!! return if (kotlinConstructor != null) propertiesForSerializationFromConstructor(kotlinConstructor, type, factory) else propertiesForSerializationFromAbstract(clazz, type, factory) } fun isConcrete(clazz: Class<*>): Boolean = !(clazz.isInterface || Modifier.isAbstract(clazz.modifiers)) -private fun propertiesForSerializationFromConstructor(kotlinConstructor: KFunction, type: Type, factory: SerializerFactory): Collection { +private fun propertiesForSerializationFromConstructor( + kotlinConstructor: KFunction, + type: Type, + factory: SerializerFactory): ConstructorDestructorMethods { val clazz = (kotlinConstructor.returnType.classifier as KClass<*>).javaObjectType // Kotlin reflection doesn't work with Java getters the way you might expect, so we drop back to good ol' beans. - val properties = Introspector.getBeanInfo(clazz).propertyDescriptors.filter { it.name != "class" }.groupBy { it.name }.mapValues { it.value[0] } + val properties = Introspector.getBeanInfo(clazz).propertyDescriptors + .filter { it.name != "class" } + .groupBy { it.name } + .mapValues { it.value[0] } + + if (properties.isNotEmpty() && kotlinConstructor.parameters.isEmpty()) { + return propertiesForSerializationFromGetters(properties, type, factory) + } + val rc: MutableList = ArrayList(kotlinConstructor.parameters.size) + for (param in kotlinConstructor.parameters) { val name = param.name ?: throw NotSerializableException("Constructor parameter of $clazz has no name.") @@ -103,7 +121,37 @@ private fun propertiesForSerializationFromConstructor(kotlinConstructo throw NotSerializableException("Property type $returnType for $name of $clazz differs from constructor parameter type ${param.type.javaType}") } } - return rc + + return ConstructorDestructorMethods(rc, emptyList()) +} + +/** + * If we determine a class has a constructor that takes no parameters then check for pairs of getters / setters + * and use those + */ +private fun propertiesForSerializationFromGetters( + properties : Map, + type: Type, + factory: SerializerFactory): ConstructorDestructorMethods { + val getters : MutableList = ArrayList(properties.size) + val setters : MutableList = ArrayList(properties.size) + + properties.forEach { property -> + val getter: Method? = property.value.readMethod + val setter: Method? = property.value.writeMethod + + if (getter == null || setter == null) return@forEach + + // NOTE: For now we're ignoring type mismatches. I.e. where the setter or getter type differs + // from the underlying property as in this case the BEAN inspector doesn't return them as a property + // and thus they arne't visible here + + getters += PropertySerializer.make(property.key, getter, resolveTypeVariables(getter.genericReturnType, type), + factory) + setters += setter + } + + return ConstructorDestructorMethods(getters, setters) } private fun constructorParamTakesReturnTypeOfGetter(getterReturnType: Type, rawGetterReturnType: Type, param: KParameter): Boolean { @@ -111,7 +159,7 @@ private fun constructorParamTakesReturnTypeOfGetter(getterReturnType: Type, rawG return typeToken.isSupertypeOf(getterReturnType) || typeToken.isSupertypeOf(rawGetterReturnType) } -private fun propertiesForSerializationFromAbstract(clazz: Class<*>, type: Type, factory: SerializerFactory): Collection { +private fun propertiesForSerializationFromAbstract(clazz: Class<*>, type: Type, factory: SerializerFactory): ConstructorDestructorMethods { // Kotlin reflection doesn't work with Java getters the way you might expect, so we drop back to good ol' beans. val properties = Introspector.getBeanInfo(clazz).propertyDescriptors.filter { it.name != "class" }.sortedBy { it.name }.filterNot { it is IndexedPropertyDescriptor } val rc: MutableList = ArrayList(properties.size) @@ -121,7 +169,7 @@ private fun propertiesForSerializationFromAbstract(clazz: Class<*>, type: Type, val returnType = resolveTypeVariables(getter.genericReturnType, type) rc += PropertySerializer.make(property.name, getter, returnType, factory) } - return rc + return ConstructorDestructorMethods(rc, emptyList()) } internal fun interfacesForSerialization(type: Type, serializerFactory: SerializerFactory): List { diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutput.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutput.kt index 2c0c4ece9f..901df19ac2 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutput.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutput.kt @@ -8,6 +8,10 @@ import java.nio.ByteBuffer import java.util.* import kotlin.collections.LinkedHashSet +data class BytesAndSchemas( + val obj: SerializedBytes, + val schema: Schema, + val transformsSchema: TransformsSchema) /** * Main entry point for serializing an object to AMQP. @@ -35,6 +39,18 @@ open class SerializationOutput(internal val serializerFactory: SerializerFactory } } + + @Throws(NotSerializableException::class) + fun serializeAndReturnSchema(obj: T): BytesAndSchemas { + try { + val blob = _serialize(obj) + val schema = Schema(schemaHistory.toList()) + return BytesAndSchemas(blob, schema, TransformsSchema.build(schema, serializerFactory)) + } finally { + andFinally() + } + } + internal fun andFinally() { objectHistory.clear() serializerHistory.clear() diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/custom/ThrowableSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/custom/ThrowableSerializer.kt index e78cf2b3d8..5700c08be7 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/custom/ThrowableSerializer.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/custom/ThrowableSerializer.kt @@ -24,7 +24,7 @@ class ThrowableSerializer(factory: SerializerFactory) : CustomSerializer.Proxy post.getA()).isInstanceOf(NullPointerException.class); + } + + @Test + public void typeMistmatch2() throws NotSerializableException { + SerializerFactory factory1 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader()); + + TypeMismatch2 tm = new TypeMismatch2(); + tm.setA("10"); + assertEquals((Integer)10, tm.getA()); + + TypeMismatch2 post = new DeserializationInput(factory1).deserialize(new SerializationOutput(factory1).serialize(tm), + TypeMismatch2.class); + + // because there is a type mismatch in the class, it won't return that info as a BEAN property and thus + // we won't serialise it and thus on deserialization it won't be initialized + assertEquals(null, post.getA()); + } +} diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPTestUtils.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPTestUtils.kt index 43fe82a6ee..08d7af6ebb 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPTestUtils.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/AMQPTestUtils.kt @@ -30,20 +30,3 @@ class TestSerializationOutput( fun testName(): String = Thread.currentThread().stackTrace[2].methodName -data class BytesAndSchemas( - val obj: SerializedBytes, - val schema: Schema, - val transformsSchema: TransformsSchema) - -// Extension for the serialize routine that returns the scheme encoded into the -// bytes as well as the bytes for simple testing -@Throws(NotSerializableException::class) -fun SerializationOutput.serializeAndReturnSchema(obj: T): BytesAndSchemas { - try { - val blob = _serialize(obj) - val schema = Schema(schemaHistory.toList()) - return BytesAndSchemas(blob, schema, TransformsSchema.build(schema, serializerFactory)) - } finally { - andFinally() - } -}