diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/PropertySerializers.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/PropertySerializers.kt index b2cfcc9311..9029dd171b 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/PropertySerializers.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/PropertySerializers.kt @@ -123,12 +123,18 @@ abstract class PropertyAccessor( class PropertyAccessorGetterSetter( initialPosition: Int, getter: PropertySerializer, - private val setter: Method?) : PropertyAccessor(initialPosition, getter) { + private val setter: Method) : PropertyAccessor(initialPosition, getter) { + init { + /** + * Play nicely with Java interop, public methods aren't marked as accessible + */ + setter.isAccessible = true + } /** * Invokes the setter on the underlying object passing in the serialized value. */ override fun set(instance: Any, obj: Any?) { - setter?.invoke(instance, *listOf(obj).toTypedArray()) + setter.invoke(instance, *listOf(obj).toTypedArray()) } } 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 b10344353f..1fffe8f20a 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 @@ -89,12 +89,18 @@ fun isConcrete(clazz: Class<*>): Boolean = !(clazz.isInterface || Modifier.isAbs * @property getter the method of a class that returns a fields value. Determined by * locating a function named getXyz for the property named in field as xyz. */ -data class PropertyDescriptor(var field: Field?, var setter: Method?, var getter: Method?) { +data class PropertyDescriptor(var field: Field?, var setter: Method?, var getter: Method?, var iser: Method?) { override fun toString() = StringBuilder("").apply { appendln("Property - ${field?.name ?: "null field"}\n") appendln(" getter - ${getter?.name ?: "no getter"}") - append(" setter - ${setter?.name ?: "no setter"}") + appendln(" setter - ${setter?.name ?: "no setter"}") + appendln(" iser - ${iser?.name ?: "no isXYZ defined"}") }.toString() + + constructor() : this(null, null, null, null) + constructor(field: Field?) : this(field, null, null, null) + + fun preferredGetter() : Method? = getter ?: iser } object PropertyDescriptorsRegex { @@ -122,12 +128,10 @@ fun Class.propertyDescriptors(): Map { val classProperties = mutableMapOf() var clazz: Class? = this - do { - // get the properties declared on this instance of class - clazz!!.declaredFields.forEach { classProperties.put(it.name, PropertyDescriptor(it, null, null)) } + clazz!!.declaredFields.forEach { classProperties.put(it.name, PropertyDescriptor(it)) } - // then pair them up with the declared getter and setter - // Note: It is possible for a class to have multiple instancess of a function where the types + do { + // Note: It is possible for a class to have multiple instances of a function where the types // differ. For example: // interface I { val a: T } // class D(override val a: String) : I @@ -138,45 +142,45 @@ fun Class.propertyDescriptors(): Map { // // In addition, only getters that take zero parameters and setters that take a single // parameter will be considered - clazz.declaredMethods?.map { func -> + clazz!!.declaredMethods?.map { func -> + if (!Modifier.isPublic(func.modifiers)) return@map + if (func.name == "getClass") return@map + PropertyDescriptorsRegex.re.find(func.name)?.apply { - try { - classProperties.getValue(groups[2]!!.value.decapitalize()).apply { - when (groups[1]!!.value) { - "set" -> { - if (func.parameterCount == 1) { - if (setter == null) setter = func - else if (TypeToken.of(setter!!.genericReturnType).isSupertypeOf(func.genericReturnType)) { - setter = func - } + // take into account those constructor properties that don't directly map to a named + // property which are, by default, already added to the map + classProperties.computeIfAbsent(groups[2]!!.value.decapitalize()) { PropertyDescriptor() }.apply { + when (groups[1]!!.value) { + "set" -> { + if (func.parameterCount == 1) { + if (setter == null) setter = func + else if (TypeToken.of(setter!!.genericReturnType).isSupertypeOf(func.genericReturnType)) { + setter = func } } - "get" -> { - if (func.parameterCount == 0) { - if (getter == null) getter = func - else if (TypeToken.of(getter!!.genericReturnType).isSupertypeOf(func.genericReturnType)) { - getter = func - } + } + "get" -> { + if (func.parameterCount == 0) { + if (getter == null) getter = func + else if (TypeToken.of(getter!!.genericReturnType).isSupertypeOf(func.genericReturnType)) { + getter = func } } - "is" -> { - if (func.parameterCount == 0) { - val rtnType = TypeToken.of(func.genericReturnType) - if ((rtnType == TypeToken.of(Boolean::class.java)) - || (rtnType == TypeToken.of(Boolean::class.javaObjectType))) { - if (getter == null) getter = func - } + } + "is" -> { + if (func.parameterCount == 0) { + val rtnType = TypeToken.of(func.genericReturnType) + if ((rtnType == TypeToken.of(Boolean::class.java)) + || (rtnType == TypeToken.of(Boolean::class.javaObjectType))) { + if (iser == null) iser = func } } } } - } catch (e: NoSuchElementException) { - // handles the getClass case from java.lang.Object - return@apply } } } - clazz = clazz?.superclass + clazz = clazz.superclass } while (clazz != null) return classProperties @@ -260,7 +264,7 @@ fun propertiesForSerializationFromSetters( return mutableListOf().apply { var idx = 0 properties.forEach { property -> - val getter: Method? = property.value.getter + val getter: Method? = property.value.preferredGetter() val setter: Method? = property.value.setter if (getter == null || setter == null) return@forEach @@ -270,13 +274,20 @@ fun propertiesForSerializationFromSetters( "takes too many arguments") } - val setterType = setter.parameterTypes.getOrNull(0)!! + val setterType = setter.parameterTypes[0]!! + if (!(TypeToken.of(property.value.field?.genericType!!).isSupertypeOf(setterType))) { throw NotSerializableException("Defined setter for parameter ${property.value.field?.name} " + "takes parameter of type $setterType yet underlying type is " + "${property.value.field?.genericType!!}") } + // make sure the setter returns the same type (within inheritance bounds) the getter accepts + if (!(TypeToken.of (setterType).isSupertypeOf(getter.returnType))) { + throw NotSerializableException("Defined setter for parameter ${property.value.field?.name} " + + "takes parameter of type $setterType yet the defined getter returns a value of type " + + "${getter.returnType}") + } this += PropertyAccessorGetterSetter( idx++, PropertySerializer.make(property.value.field!!.name, PublicPropertyReader(getter), 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 index bb710a841e..defd86d948 100644 --- 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 @@ -286,12 +286,8 @@ public class SetterConstructorTests { 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); + Assertions.assertThatThrownBy(() -> new SerializationOutput(factory1).serialize(tm)).isInstanceOf ( + NotSerializableException.class); } @Test @@ -306,11 +302,7 @@ public class SetterConstructorTests { 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()); + Assertions.assertThatThrownBy(() -> new SerializationOutput(factory1).serialize(tm)).isInstanceOf( + NotSerializableException.class); } }