From f4ad8d3e70547c6ff6804355ec911bb09cbb309a Mon Sep 17 00:00:00 2001 From: Katelyn Baker <katelyn.baker@r3.com> Date: Thu, 4 Jan 2018 15:36:42 +0000 Subject: [PATCH 1/3] 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<T : Any> : AMQPSerializer<T>, 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<Any>?) : ObjectSerializer(clazz, factory) { // explicitly set as empty to indicate it's unused by this type of serializer - override val propertySerializers: Collection<PropertySerializer> = 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<PropertySerializer> 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<Field> { - 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<String> = interfaces.map { nameForType(it) } fun construct(properties: List<Any?>): 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<Type>, 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<PropertySerializer>, + val setters : Collection<Method?>) + /** * Code for finding the constructor we will use for deserialization. * @@ -67,18 +73,30 @@ internal fun constructorForDeserialization(type: Type): KFunction<Any>? { * 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 <T : Any> propertiesForSerialization(kotlinConstructor: KFunction<T>?, type: Type, factory: SerializerFactory): Collection<PropertySerializer> { +internal fun <T : Any> propertiesForSerialization(kotlinConstructor: KFunction<T>?, 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 <T : Any> propertiesForSerializationFromConstructor(kotlinConstructor: KFunction<T>, type: Type, factory: SerializerFactory): Collection<PropertySerializer> { +private fun <T : Any> propertiesForSerializationFromConstructor( + kotlinConstructor: KFunction<T>, + 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<PropertySerializer> = 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 <T : Any> 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<String, PropertyDescriptor>, + type: Type, + factory: SerializerFactory): ConstructorDestructorMethods { + val getters : MutableList<PropertySerializer> = ArrayList(properties.size) + val setters : MutableList<Method?> = 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<PropertySerializer> { +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<PropertySerializer> = 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<Type> { 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<T : Any>( + val obj: SerializedBytes<T>, + 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 <T : Any> serializeAndReturnSchema(obj: T): BytesAndSchemas<T> { + 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<T try { val constructor = constructorForDeserialization(obj.javaClass) val props = propertiesForSerialization(constructor, obj.javaClass, factory) - for (prop in props) { + for (prop in props.getters) { extraProperties[prop.name] = prop.readMethod!!.invoke(obj) } } catch (e: NotSerializableException) { diff --git a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/SetterConstructorTests.java b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/SetterConstructorTests.java new file mode 100644 index 0000000000..eefdd0957b --- /dev/null +++ b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/SetterConstructorTests.java @@ -0,0 +1,295 @@ +package net.corda.nodeapi.internal.serialization.amqp; + +import net.corda.core.serialization.SerializedBytes; +import net.corda.nodeapi.internal.serialization.AllWhitelist; +import org.assertj.core.api.Assertions; +import org.junit.Test; +import static org.junit.Assert.*; + +import java.io.NotSerializableException; + +public class SetterConstructorTests { + + static class C { + private int a; + private int b; + private int c; + + public int getA() { return a; } + public int getB() { return b; } + public int getC() { return c; } + + public void setA(int a) { this.a = a; } + public void setB(int b) { this.b = b; } + public void setC(int c) { this.c = c; } + } + + static class C2 { + private int a; + private int b; + private int c; + + public int getA() { return a; } + public int getB() { return b; } + public int getC() { return c; } + + public void setA(int a) { this.a = a; } + public void setB(int b) { this.b = b; } + } + + static class C3 { + private int a; + private int b; + private int c; + + public int getA() { return a; } + public int getC() { return c; } + + public void setA(int a) { this.a = a; } + public void setB(int b) { this.b = b; } + public void setC(int c) { this.c = c; } + } + + static class C4 { + private int a; + private int b; + private int c; + + public int getA() { return a; } + protected int getB() { return b; } + public int getC() { return c; } + + private void setA(int a) { this.a = a; } + public void setB(int b) { this.b = b; } + public void setC(int c) { this.c = c; } + } + + static class Inner1 { + private String a; + + public Inner1(String a) { this.a = a; } + public String getA() { return this.a; } + } + + static class Inner2 { + private Double a; + + public Double getA() { return this.a; } + public void setA(Double a) { this.a = a; } + } + + static class Outer { + private Inner1 a; + private String b; + private Inner2 c; + + public Inner1 getA() { return a; } + public String getB() { return b; } + public Inner2 getC() { return c; } + + public void setA(Inner1 a) { this.a = a; } + public void setB(String b) { this.b = b; } + public void setC(Inner2 c) { this.c = c; } + } + + static class TypeMismatch { + private Integer a; + + public void setA(Integer a) { this.a = a; } + public String getA() { return this.a.toString(); } + } + + static class TypeMismatch2 { + private Integer a; + + public void setA(String a) { this.a = Integer.parseInt(a); } + public Integer getA() { return this.a; } + } + + // despite having no constructor we should still be able to serialise an instance of C + @Test + public void serialiseC() throws NotSerializableException { + SerializerFactory factory1 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader()); + SerializationOutput ser = new SerializationOutput(factory1); + + C c1 = new C(); + c1.setA(1); + c1.setB(2); + c1.setC(3); + Schema schemas = ser.serializeAndReturnSchema(c1).component2(); + assertEquals(1, schemas.component1().size()); + assertEquals(this.getClass().getName() + "$C", schemas.component1().get(0).getName()); + + CompositeType ct = (CompositeType) schemas.component1().get(0); + + assertEquals(3, ct.getFields().size()); + assertEquals("a", ct.getFields().get(0).getName()); + assertEquals("b", ct.getFields().get(1).getName()); + assertEquals("c", ct.getFields().get(2).getName()); + + // No setter for c so should only serialise two properties + C2 c2 = new C2(); + c2.setA(1); + c2.setB(2); + schemas = ser.serializeAndReturnSchema(c2).component2(); + + assertEquals(1, schemas.component1().size()); + assertEquals(this.getClass().getName() + "$C2", schemas.component1().get(0).getName()); + + ct = (CompositeType) schemas.component1().get(0); + + // With no setter for c we should only serialise a and b into the stream + assertEquals(2, ct.getFields().size()); + assertEquals("a", ct.getFields().get(0).getName()); + assertEquals("b", ct.getFields().get(1).getName()); + + // no getter for b so shouldn't serialise it,thus only a and c should apper in the envelope + C3 c3 = new C3(); + c3.setA(1); + c3.setB(2); + c3.setC(3); + schemas = ser.serializeAndReturnSchema(c3).component2(); + + assertEquals(1, schemas.component1().size()); + assertEquals(this.getClass().getName() + "$C3", schemas.component1().get(0).getName()); + + ct = (CompositeType) schemas.component1().get(0); + + // With no setter for c we should only serialise a and b into the stream + assertEquals(2, ct.getFields().size()); + assertEquals("a", ct.getFields().get(0).getName()); + assertEquals("c", ct.getFields().get(1).getName()); + + C4 c4 = new C4(); + c4.setA(1); + c4.setB(2); + c4.setC(3); + schemas = ser.serializeAndReturnSchema(c4).component2(); + + assertEquals(1, schemas.component1().size()); + assertEquals(this.getClass().getName() + "$C4", schemas.component1().get(0).getName()); + + ct = (CompositeType) schemas.component1().get(0); + + // With non public visibility on a setter and getter for a and b, only c should be serialised + assertEquals(1, ct.getFields().size()); + assertEquals("c", ct.getFields().get(0).getName()); + } + + @Test + public void deserialiseC() throws NotSerializableException { + SerializerFactory factory1 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader()); + + C cPre1 = new C(); + + int a = 1; + int b = 2; + int c = 3; + + cPre1.setA(a); + cPre1.setB(b); + cPre1.setC(c); + + SerializedBytes bytes = new SerializationOutput(factory1).serialize(cPre1); + + C cPost1 = new DeserializationInput(factory1).deserialize(bytes, C.class); + + assertEquals(a, cPost1.a); + assertEquals(b, cPost1.b); + assertEquals(c, cPost1.c); + + C2 cPre2 = new C2(); + cPre2.setA(1); + cPre2.setB(2); + + C2 cPost2 = new DeserializationInput(factory1).deserialize(new SerializationOutput(factory1).serialize(cPre2), + C2.class); + + assertEquals(a, cPost2.a); + assertEquals(b, cPost2.b); + + // no setter for c means nothing will be serialised and thus it will have the default value of zero + // set + assertEquals(0, cPost2.c); + + C3 cPre3 = new C3(); + cPre3.setA(1); + cPre3.setB(2); + cPre3.setC(3); + + C3 cPost3 = new DeserializationInput(factory1).deserialize(new SerializationOutput(factory1).serialize(cPre3), + C3.class); + + assertEquals(a, cPost3.a); + + // no getter for b means, as before, it'll have been not set and will thus be defaulted to 0 + assertEquals(0, cPost3.b); + assertEquals(c, cPost3.c); + + C4 cPre4 = new C4(); + cPre4.setA(1); + cPre4.setB(2); + cPre4.setC(3); + + C4 cPost4 = new DeserializationInput(factory1).deserialize(new SerializationOutput(factory1).serialize(cPre4), + C4.class); + + assertEquals(0, cPost4.a); + assertEquals(0, cPost4.b); + assertEquals(c, cPost4.c); + } + + @Test + public void serialiseOuterAndInner() throws NotSerializableException { + SerializerFactory factory1 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader()); + + Inner1 i1 = new Inner1("Hello"); + Inner2 i2 = new Inner2(); + i2.setA(10.5); + + Outer o = new Outer(); + o.setA(i1); + o.setB("World"); + o.setC(i2); + + Outer post = new DeserializationInput(factory1).deserialize(new SerializationOutput(factory1).serialize(o), + Outer.class); + + assertEquals("Hello", post.a.a); + assertEquals("World", post.b); + assertEquals((Double)10.5, post.c.a); + + } + + @Test + public void typeMistmatch() throws NotSerializableException { + SerializerFactory factory1 = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader()); + + TypeMismatch tm = new TypeMismatch(); + tm.setA(10); + assertEquals("10", tm.getA()); + + TypeMismatch post = new DeserializationInput(factory1).deserialize(new SerializationOutput(factory1).serialize(tm), + TypeMismatch.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 + Assertions.assertThatThrownBy(() -> 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<T : Any>( - val obj: SerializedBytes<T>, - 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 <T : Any> SerializationOutput.serializeAndReturnSchema(obj: T): BytesAndSchemas<T> { - try { - val blob = _serialize(obj) - val schema = Schema(schemaHistory.toList()) - return BytesAndSchemas(blob, schema, TransformsSchema.build(schema, serializerFactory)) - } finally { - andFinally() - } -} From f230e2670b574f5a15f3a8077aea4e48016d6332 Mon Sep 17 00:00:00 2001 From: Katelyn Baker <katelyn.baker@r3.com> Date: Thu, 4 Jan 2018 16:16:48 +0000 Subject: [PATCH 2/3] REVIEW COMMENTS --- .../internal/serialization/amqp/ObjectSerializer.kt | 4 ++-- .../internal/serialization/amqp/SerializationHelper.kt | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) 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 bd171d9f00..fa86263665 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 @@ -76,7 +76,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS obj: List<*>, schemas: SerializationSchemas, input: DeserializationInput) : Any = ifThrowsAppend({ clazz.typeName }){ - logger.debug { "Calling construction based construction" } + logger.trace ("Calling construction based construction for ${clazz.typeName}") return construct(obj.zip(propertySerializers.getters).map { it.second.readProperty(it.first, schemas, input) }) } @@ -85,7 +85,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS obj: List<*>, schemas: SerializationSchemas, input: DeserializationInput) : Any = ifThrowsAppend({ clazz.typeName }){ - logger.debug { "Calling setter based construction" } + logger.trace ("Calling setter based construction for ${clazz.typeName}") val instance : Any = javaConstructor?.newInstance() ?: throw NotSerializableException ( "Failed to instantiate instance of object $clazz") 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 a90bf7a706..5e3e28711a 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 @@ -92,7 +92,7 @@ private fun <T : Any> propertiesForSerializationFromConstructor( .mapValues { it.value[0] } if (properties.isNotEmpty() && kotlinConstructor.parameters.isEmpty()) { - return propertiesForSerializationFromGetters(properties, type, factory) + return propertiesForSerializationFromSetters(properties, type, factory) } val rc: MutableList<PropertySerializer> = ArrayList(kotlinConstructor.parameters.size) @@ -129,7 +129,7 @@ private fun <T : Any> propertiesForSerializationFromConstructor( * 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( +private fun propertiesForSerializationFromSetters( properties : Map<String, PropertyDescriptor>, type: Type, factory: SerializerFactory): ConstructorDestructorMethods { @@ -142,9 +142,9 @@ private fun propertiesForSerializationFromGetters( 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 + // NOTE: There is no need to check return and parameter types vs the underlying type for + // the getter / setter vs property as if there is a difference then that property isn't reported + // by the BEAN inspector and thus we don't consider that case here getters += PropertySerializer.make(property.key, getter, resolveTypeVariables(getter.genericReturnType, type), factory) From 3bf84ebbd4dde676553acfa22206683aa99b1b90 Mon Sep 17 00:00:00 2001 From: Katelyn Baker <katelyn.baker@r3.com> Date: Thu, 4 Jan 2018 16:24:23 +0000 Subject: [PATCH 3/3] Review Comments --- .../internal/serialization/amqp/ObjectSerializer.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 fa86263665..6cba3bbea5 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 @@ -2,6 +2,7 @@ package net.corda.nodeapi.internal.serialization.amqp import net.corda.core.utilities.contextLogger import net.corda.core.utilities.debug +import net.corda.core.utilities.trace import net.corda.nodeapi.internal.serialization.amqp.SerializerFactory.Companion.nameForType import org.apache.qpid.proton.amqp.Symbol import org.apache.qpid.proton.codec.Data @@ -76,7 +77,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS obj: List<*>, schemas: SerializationSchemas, input: DeserializationInput) : Any = ifThrowsAppend({ clazz.typeName }){ - logger.trace ("Calling construction based construction for ${clazz.typeName}") + logger.trace { "Calling construction based construction for ${clazz.typeName}" } return construct(obj.zip(propertySerializers.getters).map { it.second.readProperty(it.first, schemas, input) }) } @@ -85,7 +86,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS obj: List<*>, schemas: SerializationSchemas, input: DeserializationInput) : Any = ifThrowsAppend({ clazz.typeName }){ - logger.trace ("Calling setter based construction for ${clazz.typeName}") + logger.trace { "Calling setter based construction for ${clazz.typeName}" } val instance : Any = javaConstructor?.newInstance() ?: throw NotSerializableException ( "Failed to instantiate instance of object $clazz") @@ -112,7 +113,7 @@ open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPS private fun generateProvides(): List<String> = interfaces.map { nameForType(it) } fun construct(properties: List<Any?>): Any { - logger.debug { "Calling constructor: '$javaConstructor' with properties '$properties'" } + logger.trace { "Calling constructor: '$javaConstructor' with properties '$properties'" } return javaConstructor?.newInstance(*properties.toTypedArray()) ?: throw NotSerializableException("Attempt to deserialize an interface: $clazz. Serialized form is invalid.")