Merge pull request #2461 from corda/corda-915-backport

CORDA-915 Backport to release breanch

.idea/compiler.xml

@@ -159,4 +159,4 @@
   <component name="JavacSettings">
     <option name="ADDITIONAL_OPTIONS_STRING" value="-parameters" />
   </component>
-</project>
+</project>

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolutionSerializer.kt

@@ -21,12 +21,11 @@ import kotlin.reflect.jvm.javaType
  * @property constructorArgs used to hold the properties as sent to the object's constructor. Passed in as a
  * pre populated array as properties not present on the old constructor must be initialised in the factory
  */
-class EvolutionSerializer(
+abstract class EvolutionSerializer(
         clazz: Type,
         factory: SerializerFactory,
-        private val oldReaders: Map<String, OldParam>,
+        protected val oldReaders: Map<String, OldParam>,
-        override val kotlinConstructor: KFunction<Any>?,
+        override val kotlinConstructor: KFunction<Any>?) : ObjectSerializer(clazz, factory) {
-        private val constructorArgs: Array<Any?>) : ObjectSerializer(clazz, factory) {
 
     // explicitly set as empty to indicate it's unused by this type of serializer
     override val propertySerializers = PropertySerializersEvolution()
@@ -35,12 +34,11 @@ class EvolutionSerializer(
      * Represents a parameter as would be passed to the constructor of the class as it was
      * when it was serialised and NOT how that class appears now
      *
-     * @param type The jvm type of the parameter
      * @param resultsIndex index into the constructor argument list where the read property
      * should be placed
      * @param property object to read the actual property value
      */
-    data class OldParam(val type: Type, var resultsIndex: Int, val property: PropertySerializer) {
+    data class OldParam(var resultsIndex: Int, val property: PropertySerializer) {
         fun readProperty(obj: Any?, schemas: SerializationSchemas, input: DeserializationInput, new: Array<Any?>) =
                 property.readProperty(obj, schemas, input).apply {
                     if(resultsIndex >= 0) {
@@ -62,9 +60,10 @@ class EvolutionSerializer(
          */
         private fun getEvolverConstructor(type: Type, oldArgs: Map<String, OldParam>): KFunction<Any>? {
             val clazz: Class<*> = type.asClass()!!
+
             if (!isConcrete(clazz)) return null
 
-            val oldArgumentSet = oldArgs.map { Pair(it.key as String?, it.value.type) }
+            val oldArgumentSet = oldArgs.map { Pair(it.key as String?, it.value.property.resolvedType) }
 
             var maxConstructorVersion = Integer.MIN_VALUE
             var constructor: KFunction<Any>? = null
@@ -82,6 +81,36 @@ class EvolutionSerializer(
             return constructor ?: constructorForDeserialization(type)
         }
 
+        private fun makeWithConstructor(
+                new: ObjectSerializer,
+                factory: SerializerFactory,
+                constructor: KFunction<Any>,
+                readersAsSerialized: Map<String, OldParam>): AMQPSerializer<Any> {
+            val constructorArgs = arrayOfNulls<Any?>(constructor.parameters.size)
+
+            constructor.parameters.withIndex().forEach {
+                readersAsSerialized.get(it.value.name!!)?.apply {
+                    this.resultsIndex = it.index
+                } ?: if (!it.value.type.isMarkedNullable) {
+                    throw NotSerializableException(
+                            "New parameter ${it.value.name} is mandatory, should be nullable for evolution to worK")
+                }
+            }
+            return EvolutionSerializerViaConstructor (new.type, factory, readersAsSerialized, constructor, constructorArgs)
+        }
+
+        private fun makeWithSetters(
+                new: ObjectSerializer,
+                factory: SerializerFactory,
+                constructor: KFunction<Any>,
+                readersAsSerialized: Map<String, OldParam>,
+                classProperties: Map<String, PropertyDescriptor>): AMQPSerializer<Any> {
+            val setters = propertiesForSerializationFromSetters(classProperties,
+                    new.type,
+                    factory).associateBy({ it.getter.name }, { it })
+            return EvolutionSerializerViaSetters (new.type, factory, readersAsSerialized, constructor, setters)
+        }
+
         /**
          * Build a serialization object for deserialization only of objects serialised
          * as different versions of a class.
@@ -95,46 +124,43 @@ class EvolutionSerializer(
          */
         fun make(old: CompositeType, new: ObjectSerializer,
                  factory: SerializerFactory): AMQPSerializer<Any> {
-
+            // The order in which the properties were serialised is important and must be preserved
-            val readersAsSerialized = linkedMapOf<String, OldParam>(
+            val readersAsSerialized = LinkedHashMap<String, OldParam>()
-                    *(old.fields.map {
+            old.fields.forEach {
-                        val returnType =  try {
+                readersAsSerialized[it.name] = try {
-                            it.getTypeAsClass(factory.classloader)
+                    OldParam(-1, PropertySerializer.make(it.name, EvolutionPropertyReader(),
-                        } catch (e: ClassNotFoundException) {
+                            it.getTypeAsClass(factory.classloader), factory))
-                            throw NotSerializableException(e.message)
+                } catch (e: ClassNotFoundException) {
-                        }
+                    throw NotSerializableException(e.message)
-
-                        it.name to OldParam(
-                                returnType,
-                                -1,
-                                PropertySerializer.make(
-                                        it.name, PublicPropertyReader(null), returnType, factory))
-                    }.toTypedArray())
-            )
-
-            val constructor = getEvolverConstructor(new.type, readersAsSerialized) ?:
-                    throw NotSerializableException(
-                            "Attempt to deserialize an interface: ${new.type}. Serialized form is invalid.")
-
-            val constructorArgs = arrayOfNulls<Any?>(constructor.parameters.size)
-
-            constructor.parameters.withIndex().forEach {
-                readersAsSerialized.get(it.value.name!!)?.apply {
-                    this.resultsIndex = it.index
-                } ?: if (!it.value.type.isMarkedNullable) {
-                    throw NotSerializableException(
-                            "New parameter ${it.value.name} is mandatory, should be nullable for evolution to worK")
                 }
             }
 
-            return EvolutionSerializer(new.type, factory, readersAsSerialized, constructor, constructorArgs)
+            // cope with the situation where a generic interface was serialised as a type, in such cases
+            // return the synthesised object which is, given the absence of a constructor, a no op
+            val constructor = getEvolverConstructor(new.type, readersAsSerialized) ?: return new
+
+            val classProperties = new.type.asClass()?.propertyDescriptors() ?: emptyMap()
+
+            return if (classProperties.isNotEmpty() && constructor.parameters.isEmpty()) {
+                makeWithSetters(new, factory, constructor, readersAsSerialized, classProperties)
+            }
+            else {
+                makeWithConstructor(new, factory, constructor, readersAsSerialized)
+            }
         }
     }
 
-    override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput, offset: Int) {
+    override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput, debugIndent: Int) {
         throw UnsupportedOperationException("It should be impossible to write an evolution serializer")
     }
+}
 
+class EvolutionSerializerViaConstructor(
+        clazz: Type,
+        factory: SerializerFactory,
+        oldReaders: Map<String, EvolutionSerializer.OldParam>,
+        kotlinConstructor: KFunction<Any>?,
+        private val constructorArgs: Array<Any?>) : EvolutionSerializer (clazz, factory, oldReaders, kotlinConstructor) {
     /**
      * Unlike a normal [readObject] call where we simply apply the parameter deserialisers
      * to the object list of values we need to map that list, which is ordered per the
@@ -151,7 +177,36 @@ class EvolutionSerializer(
         oldReaders.values.zip(obj).map { it.first.readProperty(it.second, schemas, input, constructorArgs) }
 
         return javaConstructor?.newInstance(*(constructorArgs)) ?:
-                throw NotSerializableException("Attempt to deserialize an interface: $clazz. Serialized form is invalid.")
+                throw NotSerializableException(
+                        "Attempt to deserialize an interface: $clazz. Serialized form is invalid.")
+    }
+}
+
+/**
+ * Specific instance of an [EvolutionSerializer] where the properties of the object are set via calling
+ * named setter functions on the instantiated object.
+ */
+class EvolutionSerializerViaSetters(
+        clazz: Type,
+        factory: SerializerFactory,
+        oldReaders: Map<String, EvolutionSerializer.OldParam>,
+        kotlinConstructor: KFunction<Any>?,
+        private val setters: Map<String, PropertyAccessor>) : EvolutionSerializer (clazz, factory, oldReaders, kotlinConstructor) {
+
+    override fun readObject(obj: Any, schemas: SerializationSchemas, input: DeserializationInput): Any {
+        if (obj !is List<*>) throw NotSerializableException("Body of described type is unexpected $obj")
+
+        val instance : Any = javaConstructor?.newInstance() ?: throw NotSerializableException (
+                "Failed to instantiate instance of object $clazz")
+
+        // *must* read all the parameters in the order they were serialized
+        oldReaders.values.zip(obj).forEach {
+            // if that property still exists on the new object then set it
+            it.first.property.readProperty(it.second, schemas, input).apply {
+                setters[it.first.property.name]?.set(instance, this)
+            }
+        }
+        return instance
     }
 }
 
@@ -177,12 +232,24 @@ class EvolutionSerializerGetter : EvolutionSerializerGetterBase() {
     override fun getEvolutionSerializer(factory: SerializerFactory,
                                         typeNotation: TypeNotation,
                                         newSerializer: AMQPSerializer<Any>,
-                                        schemas: SerializationSchemas): AMQPSerializer<Any> =
+                                        schemas: SerializationSchemas): AMQPSerializer<Any> {
-            factory.getSerializersByDescriptor().computeIfAbsent(typeNotation.descriptor.name!!) {
+        return factory.getSerializersByDescriptor().computeIfAbsent(typeNotation.descriptor.name!!) {
-                when (typeNotation) {
+            when (typeNotation) {
-                    is CompositeType -> EvolutionSerializer.make(typeNotation, newSerializer as ObjectSerializer, factory)
+                is CompositeType -> EvolutionSerializer.make(typeNotation, newSerializer as ObjectSerializer, factory)
-                    is RestrictedType -> EnumEvolutionSerializer.make(typeNotation, newSerializer, factory, schemas)
+                is RestrictedType -> {
+                    // The fingerprint of a generic collection can be changed through bug fixes to the
+                    // fingerprinting function making it appear as if the class has altered whereas it hasn't.
+                    // Given we don't support the evolution of these generic containers, if it appears
+                    // one has been changed, simply return the original serializer and associate it with
+                    // both the new and old fingerprint
+                    if (newSerializer is CollectionSerializer || newSerializer is MapSerializer) {
+                        newSerializer
+                    } else {
+                        EnumEvolutionSerializer.make(typeNotation, newSerializer, factory, schemas)
+                    }
                 }
             }
+        }
+    }
 }
 

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/PropertySerializers.kt

@@ -14,6 +14,9 @@ abstract class PropertyReader {
     abstract fun isNullable(): Boolean
 }
 
+/**
+ * Accessor for those properties of a class that have defined getter functions.
+ */
 class PublicPropertyReader(private val readMethod: Method?) : PropertyReader() {
     init {
         readMethod?.isAccessible = true
@@ -43,6 +46,10 @@ class PublicPropertyReader(private val readMethod: Method?) : PropertyReader() {
     override fun isNullable(): Boolean = readMethod?.returnsNullable() ?: false
 }
 
+/**
+ * Accessor for those properties of a class that do not have defined getter functions. In which case
+ * we used reflection to remove the unreadable status from that property whilst it's accessed.
+ */
 class PrivatePropertyReader(val field: Field, parentType: Type) : PropertyReader() {
     init {
         loggerFor<PropertySerializer>().warn("Create property Serializer for private property '${field.name}' not "
@@ -68,6 +75,20 @@ class PrivatePropertyReader(val field: Field, parentType: Type) : PropertyReader
     }
 }
 
+/**
+ * Special instance of a [PropertyReader] for use only by [EvolutionSerializer]s to make
+ * it explicit that no properties are ever actually read from an object as the evolution
+ * serializer should only be accessing the already serialized form.
+ */
+class EvolutionPropertyReader : PropertyReader() {
+    override fun read(obj: Any?): Any? {
+        throw UnsupportedOperationException("It should be impossible for an evolution serializer to "
+                + "be reading from an object")
+    }
+
+    override fun isNullable() = true
+}
+
 /**
  * Represents a generic interface to a serializable property of an object.
  *
@@ -102,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())
     }
 }
 

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt

@@ -460,6 +460,6 @@ private fun fingerprintForObject(
                         .putUnencodedChars(prop.getter.name)
                         .putUnencodedChars(if (prop.getter.mandatory) NOT_NULLABLE_HASH else NULLABLE_HASH)
             }
-    interfacesForSerialization(type, factory).map { fingerprintForType(it, type, alreadySeen, hasher, factory, debugIndent+4) }
+    interfacesForSerialization(type, factory).map { fingerprintForType(it, type, alreadySeen, hasher, factory, debugIndent+1) }
     return hasher
 }

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt

@@ -6,11 +6,9 @@ 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.lang.reflect.Field
 import java.util.*
 import kotlin.reflect.KClass
 import kotlin.reflect.KFunction
@@ -72,62 +70,184 @@ internal fun constructorForDeserialization(type: Type): KFunction<Any>? {
 internal fun <T : Any> propertiesForSerialization(
         kotlinConstructor: KFunction<T>?,
         type: Type,
-        factory: SerializerFactory) = PropertySerializers.make (
+        factory: SerializerFactory) = PropertySerializers.make(
-            if (kotlinConstructor != null) {
+        if (kotlinConstructor != null) {
-                propertiesForSerializationFromConstructor(kotlinConstructor, type, factory)
+            propertiesForSerializationFromConstructor(kotlinConstructor, type, factory)
-            } else {
+        } else {
-                propertiesForSerializationFromAbstract(type.asClass()!!, type, factory)
+            propertiesForSerializationFromAbstract(type.asClass()!!, type, factory)
-            }.sortedWith(PropertyAccessor))
+        }.sortedWith(PropertyAccessor))
+
 
 fun isConcrete(clazz: Class<*>): Boolean = !(clazz.isInterface || Modifier.isAbstract(clazz.modifiers))
 
+/**
+ * Encapsulates the property of a class and its potential getter and setter methods.
+ *
+ * @property field a property of a class.
+ * @property setter the method of a class that sets the field. Determined by locating
+ * a function called setXyz on the class for the property named in field as xyz.
+ * @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?, var iser: Method?) {
+    override fun toString() = StringBuilder("").apply {
+        appendln("Property - ${field?.name ?: "null field"}\n")
+        appendln("  getter - ${getter?.name ?: "no getter"}")
+        appendln("  setter - ${setter?.name ?: "no setter"}")
+        appendln("  iser   - ${iser?.name ?: "no isXYZ defined"}")
+    }.toString()
+
+    constructor() : this(null, null, null, null)
+
+    fun preferredGetter() : Method? = getter ?: iser
+}
+
+object PropertyDescriptorsRegex {
+    // match an uppercase letter that also has a corresponding lower case equivalent
+    val re = Regex("(?<type>get|set|is)(?<var>\\p{Lu}.*)")
+}
+
+/**
+ * Collate the properties of a class and match them with their getter and setter
+ * methods as per a JavaBean.
+ *
+ * for a property
+ *      exampleProperty
+ *
+ * We look for methods
+ *      setExampleProperty
+ *      getExampleProperty
+ *      isExampleProperty
+ *
+ * Where setExampleProperty must return a type compatible with exampleProperty, getExampleProperty must
+ * take a single parameter of a type compatible with exampleProperty and isExampleProperty must
+ * return a boolean
+ */
+fun Class<out Any?>.propertyDescriptors(): Map<String, PropertyDescriptor> {
+    val classProperties = mutableMapOf<String, PropertyDescriptor>()
+
+    var clazz: Class<out Any?>? = this
+
+    do {
+        clazz!!.declaredFields.forEach { property ->
+            classProperties.computeIfAbsent(property.name) {
+                PropertyDescriptor()
+            }.apply {
+                this.field = property
+            }
+        }
+
+        // Note: It is possible for a class to have multiple instances of a function where the types
+        // differ. For example:
+        //      interface I<out T> { val a: T }
+        //      class D(override val a: String) : I<String>
+        // instances of D will have both
+        //      getA - returning a String (java.lang.String) and
+        //      getA - returning an Object (java.lang.Object)
+        // In this instance we take the most derived object
+        //
+        // In addition, only getters that take zero parameters and setters that take a single
+        // parameter will be considered
+        clazz.declaredMethods?.map { func ->
+            if (!Modifier.isPublic(func.modifiers)) return@map
+            if (func.name == "getClass") return@map
+
+            PropertyDescriptorsRegex.re.find(func.name)?.apply {
+                // 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
+                                }
+                            }
+                        }
+                        "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
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+        clazz = clazz.superclass
+    } while (clazz != null)
+
+    return classProperties
+}
+
+/**
+ * From a constructor, determine which properties of a class are to be serialized.
+ *
+ * @param kotlinConstructor The constructor to be used to instantiate instances of the class
+ * @param type The class's [Type]
+ * @param factory The factory generating the serializer wrapping this function.
+ */
 internal fun <T : Any> propertiesForSerializationFromConstructor(
         kotlinConstructor: KFunction<T>,
         type: Type,
         factory: SerializerFactory): List<PropertyAccessor> {
     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] }
 
-    if (properties.isNotEmpty() && kotlinConstructor.parameters.isEmpty()) {
+    val classProperties = clazz.propertyDescriptors()
-        return propertiesForSerializationFromSetters(properties, type, factory)
+
+    if (classProperties.isNotEmpty() && kotlinConstructor.parameters.isEmpty()) {
+        return propertiesForSerializationFromSetters(classProperties, type, factory)
     }
 
     return mutableListOf<PropertyAccessor>().apply {
         kotlinConstructor.parameters.withIndex().forEach { param ->
-            val name = param.value.name ?: throw NotSerializableException("Constructor parameter of $clazz has no name.")
+            val name = param.value.name ?: throw NotSerializableException(
+                    "Constructor parameter of $clazz has no name.")
 
-            val propertyReader = if (name in properties) {
+            val propertyReader = if (name in classProperties) {
-                // it's a publicly accessible property
+                if (classProperties[name]!!.getter != null) {
-                val matchingProperty = properties[name]!!
+                    // it's a publicly accessible property
+                    val matchingProperty = classProperties[name]!!
 
-                // Check that the method has a getter in java.
+                    // Check that the method has a getter in java.
-                val getter = matchingProperty.readMethod ?: throw NotSerializableException(
+                    val getter = matchingProperty.getter ?: throw NotSerializableException(
-                        "Property has no getter method for $name of $clazz. If using Java and the parameter name"
+                            "Property has no getter method for $name of $clazz. If using Java and the parameter name"
-                                + "looks anonymous, check that you have the -parameters option specified in the Java compiler."
+                                    + "looks anonymous, check that you have the -parameters option specified in the "
-                                + "Alternately, provide a proxy serializer (SerializationCustomSerializer) if "
+                                    + "Java compiler. Alternately, provide a proxy serializer "
-                                + "recompiling isn't an option.")
+                                    + "(SerializationCustomSerializer) if recompiling isn't an option.")
 
-                val returnType = resolveTypeVariables(getter.genericReturnType, type)
+                    val returnType = resolveTypeVariables(getter.genericReturnType, type)
-                if (!constructorParamTakesReturnTypeOfGetter(returnType, getter.genericReturnType, param.value)) {
+                    if (!constructorParamTakesReturnTypeOfGetter(returnType, getter.genericReturnType, param.value)) {
-                    throw NotSerializableException(
+                        throw NotSerializableException(
-                            "Property type '$returnType' for '$name' of '$clazz' differs from constructor parameter "
+                                "Property '$name' has type '$returnType' on class '$clazz' but differs from constructor " +
-                                    + "type '${param.value.type.javaType}'")
+                                "parameter type '${param.value.type.javaType}'")
-                }
+                    }
+
+                    Pair(PublicPropertyReader(getter), returnType)
+                } else {
+                    val field = classProperties[name]!!.field ?:
+                        throw NotSerializableException("No property matching constructor parameter named '$name' " +
+                                "of '$clazz'. If using Java, check that you have the -parameters option specified " +
+                                "in the Java compiler. Alternately, provide a proxy serializer " +
+                                "(SerializationCustomSerializer) if recompiling isn't an option")
 
-                Pair(PublicPropertyReader(getter), returnType)
-            } else {
-                try {
-                    val field = clazz.getDeclaredField(param.value.name)
                     Pair(PrivatePropertyReader(field, type), field.genericType)
-                } catch (e: NoSuchFieldException) {
-                    throw NotSerializableException("No property matching constructor parameter named '$name' of '$clazz'. " +
-                            "If using Java, check that you have the -parameters option specified in the Java compiler. " +
-                            "Alternately, provide a proxy serializer (SerializationCustomSerializer) if recompiling isn't an option")
                 }
+            } else {
+                throw NotSerializableException(
+                        "Constructor parameter $name doesn't refer to a property of class '$clazz'")
             }
 
             this += PropertyAccessorConstructor(
@@ -141,23 +261,40 @@ internal 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 propertiesForSerializationFromSetters(
+fun propertiesForSerializationFromSetters(
         properties: Map<String, PropertyDescriptor>,
         type: Type,
         factory: SerializerFactory): List<PropertyAccessor> {
     return mutableListOf<PropertyAccessorGetterSetter>().apply {
         var idx = 0
+
         properties.forEach { property ->
-            val getter: Method? = property.value.readMethod
+            val getter: Method? = property.value.preferredGetter()
-            val setter: Method? = property.value.writeMethod
+            val setter: Method? = property.value.setter
 
             if (getter == null || setter == null) return@forEach
 
-            // NOTE: There is no need to check return and parameter types vs the underlying type for
+            if (setter.parameterCount != 1) {
-            // the getter / setter vs property as if there is a difference then that property isn't reported
+                throw NotSerializableException("Defined setter for parameter ${property.value.field?.name} " +
-            // by the BEAN inspector and thus we don't consider that case here
+                        "takes too many arguments")
+            }
 
-            this += PropertyAccessorGetterSetter (
+            val setterType = setter.parameterTypes[0]!!
+
+            if ((property.value.field != null) &&
+                    (!(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.key, PublicPropertyReader(getter),
                             resolveTypeVariables(getter.genericReturnType, type), factory),
@@ -186,23 +323,19 @@ private fun propertiesForSerializationFromAbstract(
         clazz: Class<*>,
         type: Type,
         factory: SerializerFactory): List<PropertyAccessor> {
-    // Kotlin reflection doesn't work with Java getters the way you might expect, so we drop back to good ol' beans.
+    val properties = clazz.propertyDescriptors()
-    val properties = Introspector.getBeanInfo(clazz).propertyDescriptors
-            .filter { it.name != "class" }
-            .sortedBy { it.name }
-            .filterNot { it is IndexedPropertyDescriptor }
 
-   return mutableListOf<PropertyAccessorConstructor>().apply {
+    return mutableListOf<PropertyAccessorConstructor>().apply {
-       properties.withIndex().forEach { property ->
+        properties.toList().withIndex().forEach {
-           // Check that the method has a getter in java.
+            val getter = it.value.second.getter ?: return@forEach
-           val getter = property.value.readMethod ?: throw NotSerializableException(
+            if (it.value.second.field == null) return@forEach
-                   "Property has no getter method for ${property.value.name} of $clazz.")
+
-           val returnType = resolveTypeVariables(getter.genericReturnType, type)
+            val returnType = resolveTypeVariables(getter.genericReturnType, type)
-           this += PropertyAccessorConstructor(
+            this += PropertyAccessorConstructor(
-                   property.index,
+                    it.index,
-                   PropertySerializer.make(property.value.name, PublicPropertyReader(getter), returnType, factory))
+                    PropertySerializer.make(it.value.first, PublicPropertyReader(getter), returnType, factory))
-       }
+        }
-   }
+    }
 }
 
 internal fun interfacesForSerialization(type: Type, serializerFactory: SerializerFactory): List<Type> {
@@ -270,7 +403,11 @@ private fun resolveTypeVariables(actualType: Type, contextType: Type?): Type {
     // TODO: surely we check it is concrete at this point with no TypeVariables
     return if (resolvedType is TypeVariable<*>) {
         val bounds = resolvedType.bounds
-        return if (bounds.isEmpty()) SerializerFactory.AnyType else if (bounds.size == 1) resolveTypeVariables(bounds[0], contextType) else throw NotSerializableException("Got bounded type $actualType but only support single bound.")
+        return if (bounds.isEmpty()) {
+            SerializerFactory.AnyType
+        } else if (bounds.size == 1) {
+            resolveTypeVariables(bounds[0], contextType)
+        } else throw NotSerializableException("Got bounded type $actualType but only support single bound.")
     } else {
         resolvedType
     }

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt

@@ -120,16 +120,17 @@ val typeStrToType: Map<Pair<String, Boolean>, Class<out Any?>> = mapOf(
         Pair("byte", false) to Byte::class.javaObjectType
 )
 
+fun String.stripGenerics() : String = if(this.endsWith('>')) {
+    this.substring(0, this.indexOf('<'))
+} else this
+
 fun AMQPField.getTypeAsClass(classloader: ClassLoader) = typeStrToType[Pair(type, mandatory)] ?: when (type) {
     "string" -> String::class.java
     "binary" -> ByteArray::class.java
-    "*" -> if (requires.isEmpty()) Any::class.java else classloader.loadClass(requires[0])
+    "*" -> if (requires.isEmpty()) Any::class.java else {
-    else -> {
+        classloader.loadClass(requires[0].stripGenerics())
-        classloader.loadClass(
-        if (type.endsWith("?>")) {
-            type.substring(0, type.indexOf('<'))
-        } else type)
     }
+    else -> classloader.loadClass(type.stripGenerics())
 }
 
 fun AMQPField.validateType(classloader: ClassLoader) = when (type) {

node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/JavaPrivatePropertyTests.java

@@ -23,6 +23,115 @@ public class JavaPrivatePropertyTests {
         public String getA() { return a; }
     }
 
+    static class B {
+        private Boolean b;
+
+        B(Boolean b) { this.b = b; }
+
+        public Boolean isB() {
+            return this.b;
+        }
+    }
+
+    static class B2 {
+        private Boolean b;
+
+        public Boolean isB() {
+            return this.b;
+        }
+
+        public void setB(Boolean b) {
+            this.b = b;
+        }
+    }
+
+    static class B3 {
+        private Boolean b;
+
+        // break the BEAN format explicitly (i.e. it's not isB)
+        public Boolean isb() {
+            return this.b;
+        }
+
+        public void setB(Boolean b) {
+            this.b = b;
+        }
+    }
+
+    static class C3 {
+        private Integer a;
+
+        public Integer getA() {
+            return this.a;
+        }
+
+        public Boolean isA() {
+            return this.a > 0;
+        }
+
+        public void setA(Integer a) {
+            this.a = a;
+        }
+    }
+
+    @Test
+    public void singlePrivateBooleanWithConstructor() throws NotSerializableException, NoSuchFieldException, IllegalAccessException {
+        EvolutionSerializerGetterBase evolutionSerializerGetter = new EvolutionSerializerGetter();
+        SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(),
+                evolutionSerializerGetter);
+        SerializationOutput ser = new SerializationOutput(factory);
+        DeserializationInput des = new DeserializationInput(factory);
+
+        B b = new B(true);
+        B b2 = des.deserialize(ser.serialize(b), B.class);
+        assertEquals (b.b, b2.b);
+    }
+
+    @Test
+    public void singlePrivateBooleanWithNoConstructor() throws NotSerializableException, NoSuchFieldException, IllegalAccessException {
+        EvolutionSerializerGetterBase evolutionSerializerGetter = new EvolutionSerializerGetter();
+        SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(),
+                evolutionSerializerGetter);
+        SerializationOutput ser = new SerializationOutput(factory);
+        DeserializationInput des = new DeserializationInput(factory);
+
+        B2 b = new B2();
+        b.setB(false);
+        B2 b2 = des.deserialize(ser.serialize(b), B2.class);
+        assertEquals (b.b, b2.b);
+    }
+
+    @Test
+    public void testCapitilsationOfIs() throws NotSerializableException, NoSuchFieldException, IllegalAccessException {
+        EvolutionSerializerGetterBase evolutionSerializerGetter = new EvolutionSerializerGetter();
+        SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(),
+                evolutionSerializerGetter);
+        SerializationOutput ser = new SerializationOutput(factory);
+        DeserializationInput des = new DeserializationInput(factory);
+
+        B3 b = new B3();
+        b.setB(false);
+        B3 b2 = des.deserialize(ser.serialize(b), B3.class);
+
+        // since we can't find a getter for b (isb != isB) then we won't serialize that parameter
+        assertEquals (null, b2.b);
+    }
+
+    @Test
+    public void singlePrivateIntWithBoolean() throws NotSerializableException, NoSuchFieldException, IllegalAccessException {
+        EvolutionSerializerGetterBase evolutionSerializerGetter = new EvolutionSerializerGetter();
+        SerializerFactory factory = new SerializerFactory(AllWhitelist.INSTANCE, ClassLoader.getSystemClassLoader(),
+                evolutionSerializerGetter);
+        SerializationOutput ser = new SerializationOutput(factory);
+        DeserializationInput des = new DeserializationInput(factory);
+
+        C3 c = new C3();
+        c.setA(12345);
+        C3 c2 = des.deserialize(ser.serialize(c), C3.class);
+
+        assertEquals (c.a, c2.a);
+    }
+
     @Test
     public void singlePrivateWithConstructor() throws NotSerializableException, NoSuchFieldException, IllegalAccessException {
         EvolutionSerializerGetterBase evolutionSerializerGetter = new EvolutionSerializerGetter();

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),
+        Assertions.assertThatThrownBy(() -> new SerializationOutput(factory1).serialize(tm)).isInstanceOf (
-                TypeMismatch.class);
+                NotSerializableException.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
@@ -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),
+        Assertions.assertThatThrownBy(() -> new SerializationOutput(factory1).serialize(tm)).isInstanceOf(
-                TypeMismatch2.class);
+                NotSerializableException.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());
     }
 }

node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt

@@ -1,12 +1,21 @@
 package net.corda.nodeapi.internal.serialization.amqp
 
+import net.corda.core.crypto.Crypto.generateKeyPair
+import net.corda.core.crypto.SignedData
+import net.corda.core.crypto.sign
 import net.corda.core.serialization.DeprecatedConstructorForDeserialization
 import net.corda.core.serialization.SerializedBytes
+import net.corda.nodeapi.internal.network.NetworkParameters
+import net.corda.nodeapi.internal.network.NotaryInfo
 import net.corda.testing.common.internal.ProjectStructure.projectRootDir
+import net.corda.testing.core.DUMMY_NOTARY_NAME
+import net.corda.testing.core.TestIdentity
+import org.junit.Ignore
 import org.junit.Test
 import java.io.File
 import java.io.NotSerializableException
 import java.net.URI
+import java.time.Instant
 import kotlin.test.assertEquals
 
 // To regenerate any of the binary test files do the following
@@ -462,4 +471,112 @@ class EvolvabilityTests {
         assertEquals(f, db3.f)
         assertEquals(-1, db3.g)
     }
-}
+
+    //
+    // This test uses a NetworkParameters signed set of bytes generated by R3 Corda and
+    // is here to ensure we can still read them. This test exists because of the break in
+    // being able to deserialize an object serialized prior to some fixes to the fingerprinter.
+    //
+    // The file itself was generated from R3 Corda at commit
+    //      6a6b6f256 Skip cache invalidation during init() - caches are still null.
+    //
+    // To regenerate the file un-ignore the test below this one (regenerate broken network parameters),
+    // to regenerate at a specific version add that test to a checkout at the desired sha then take
+    // the resulting file and add to the repo, changing the filename as appropriate
+    //
+    @Test
+    fun readBrokenNetworkParameters(){
+        val sf = testDefaultFactory()
+        sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.InstantSerializer(sf))
+        sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.PublicKeySerializer)
+
+        //
+        // filename breakdown
+        // networkParams - because this is a serialised set of network parameters
+        // r3corda - generated by R3 Corda instead of Corda
+        // 6a6b6f256 - Commit sha of the build that generated the file we're testing against
+        //
+        val resource = "networkParams.r3corda.6a6b6f256"
+
+        val path = EvolvabilityTests::class.java.getResource(resource)
+        val f = File(path.toURI())
+        val sc2 = f.readBytes()
+        val deserializedC = DeserializationInput(sf).deserialize(SerializedBytes<SignedData<NetworkParameters>>(sc2))
+        val networkParams = DeserializationInput(sf).deserialize(deserializedC.raw)
+
+        assertEquals(1000, networkParams.maxMessageSize)
+        assertEquals(1000, networkParams.maxTransactionSize)
+        assertEquals(3, networkParams.minimumPlatformVersion)
+        assertEquals(1, networkParams.notaries.size)
+        assertEquals(TestIdentity(DUMMY_NOTARY_NAME, 20).party, networkParams.notaries.firstOrNull()?.identity)
+    }
+
+    //
+    // This test created a serialized and signed set of Network Parameters to test whether we
+    // can still deserialize them
+    //
+    @Test
+    @Ignore("This test simply regenerates the test file used for readBrokenNetworkParameters")
+    fun `regenerate broken network parameters`() {
+        // note: 6a6b6f256 is the sha that generates the file
+        val resource = "networkParams.<corda version>.<commit sha>"
+        val DUMMY_NOTARY = TestIdentity(DUMMY_NOTARY_NAME, 20).party
+        val networkParameters = NetworkParameters(
+                3, listOf(NotaryInfo(DUMMY_NOTARY, false)),1000, 1000, Instant.EPOCH, 1 )
+
+        val sf = testDefaultFactory()
+        sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.InstantSerializer(sf))
+        sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.PublicKeySerializer)
+
+        val testOutput = TestSerializationOutput(true, sf)
+        val serialized = testOutput.serialize(networkParameters)
+        val keyPair = generateKeyPair()
+        val sig = keyPair.private.sign(serialized.bytes, keyPair.public)
+        val signed = SignedData(serialized, sig)
+        val signedAndSerialized = testOutput.serialize(signed)
+
+        File(URI("$localPath/$resource")).writeBytes( signedAndSerialized.bytes)
+    }
+
+    @Suppress("UNCHECKED_CAST")
+    @Test
+    fun getterSetterEvolver1() {
+        val resource = "EvolvabilityTests.getterSetterEvolver1"
+        val sf = testDefaultFactory()
+
+        //
+        // Class as it was serialised
+        //
+        // data class C(var c: Int, var d: Int, var b: Int, var e: Int, var a: Int) {
+        //     // This will force the serialization engine to use getter / setter
+        //     // instantiation for the object rather than construction
+        //     @ConstructorForDeserialization
+        //     @Suppress("UNUSED")
+        //     constructor() : this(0, 0, 0, 0, 0)
+        // }
+        //
+        // File(URI("$localPath/$resource")).writeBytes(SerializationOutput(sf).serialize(C(3,4,2,5,1)).bytes)
+
+        //
+        // Class as it exists now, c has been removed
+        //
+        data class C(var d: Int, var b: Int, var e: Int, var a: Int) {
+            // This will force the serialization engine to use getter / setter
+            // instantiation for the object rather than construction
+            @ConstructorForDeserialization
+            @Suppress("UNUSED")
+            constructor() : this(0, 0, 0, 0)
+        }
+
+        val path = EvolvabilityTests::class.java.getResource(resource)
+        val f = File(path.toURI())
+
+        val sc2 = f.readBytes()
+        val deserializedC = DeserializationInput(sf).deserialize(SerializedBytes<C>(sc2))
+
+        assertEquals(1, deserializedC.a)
+        assertEquals(2, deserializedC.b)
+        assertEquals(4, deserializedC.d)
+        assertEquals(5, deserializedC.e)
+    }
+}

node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/GenericsTests.kt

@@ -501,4 +501,29 @@ class GenericsTests {
         assertEquals(state.context.a,  des1.obj.state.context.a)
     }
 
+    fun implemntsGeneric() {
+        open class B<out T>(open val a: T)
+        class D(override val a: String) : B<String>(a)
+
+        val factory = testDefaultFactoryNoEvolution()
+
+        val bytes = SerializationOutput(factory).serialize(D("Test"))
+
+        DeserializationInput(factory).deserialize(bytes).apply { assertEquals("Test", this.a) }
+    }
+
+    interface implementsGenericInterfaceI<out T> {
+        val a: T
+    }
+
+    @Test
+    fun implemntsGenericInterface() {
+        class D(override val a: String) : implementsGenericInterfaceI<String>
+
+        val factory = testDefaultFactoryNoEvolution()
+
+        val bytes = SerializationOutput(factory).serialize(D("Test"))
+
+        DeserializationInput(factory).deserialize(bytes).apply { assertEquals("Test", this.a) }
+    }
 }

node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/PrivatePropertyTests.kt

@@ -6,6 +6,8 @@ import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 import org.junit.Test
 import org.apache.qpid.proton.amqp.Symbol
+import org.assertj.core.api.Assertions
+import java.io.NotSerializableException
 import java.util.concurrent.ConcurrentHashMap
 
 class PrivatePropertyTests {
@@ -29,6 +31,15 @@ class PrivatePropertyTests {
         assertEquals(c1, c2)
     }
 
+    @Test
+    fun testWithOnePrivatePropertyBoolean() {
+        data class C(private val b: Boolean)
+
+        C(false).apply {
+            assertEquals(this, DeserializationInput(factory).deserialize(SerializationOutput(factory).serialize(this)))
+        }
+    }
+
     @Test
     fun testWithOnePrivatePropertyNullableNotNull() {
         data class C(private val b: String?)
@@ -56,6 +67,60 @@ class PrivatePropertyTests {
         assertEquals(c1, c2)
     }
 
+    @Test
+    fun testWithInheritance() {
+        open class B(val a: String, protected val b: String)
+        class D (a: String, b: String) : B (a, b) {
+            override fun equals(other: Any?): Boolean = when (other) {
+                is D -> other.a == a && other.b == b
+                else -> false
+            }
+        }
+
+        val d1 = D("clump", "lump")
+        val d2 = DeserializationInput(factory).deserialize(SerializationOutput(factory).serialize(d1))
+
+        assertEquals(d1, d2)
+    }
+
+    @Test
+    fun testMultiArgSetter() {
+        @Suppress("UNUSED")
+        data class C(private var a: Int, var b: Int) {
+            // This will force the serialization engine to use getter / setter
+            // instantiation for the object rather than construction
+            @ConstructorForDeserialization
+            constructor() : this(0, 0)
+
+            fun setA(a: Int, b: Int) { this.a = a }
+            fun getA() = a
+        }
+
+        val c1 = C(33, 44)
+        val c2 = DeserializationInput(factory).deserialize(SerializationOutput(factory).serialize(c1))
+        assertEquals(0, c2.getA())
+        assertEquals(44, c2.b)
+    }
+
+    @Test
+    fun testBadTypeArgSetter() {
+        @Suppress("UNUSED")
+        data class C(private var a: Int, val b: Int) {
+            @ConstructorForDeserialization
+            constructor() : this(0, 0)
+
+            fun setA(a: String) { this.a = a.toInt() }
+            fun getA() = a
+        }
+
+        val c1 = C(33, 44)
+        Assertions.assertThatThrownBy {
+            SerializationOutput(factory).serialize(c1)
+        }.isInstanceOf(NotSerializableException::class.java).hasMessageContaining(
+                "Defined setter for parameter a takes parameter of type class java.lang.String " +
+                        "yet underlying type is int ")
+    }
+
     @Test
     fun testWithOnePublicOnePrivateProperty2() {
         data class C(val a: Int, private val b: Int)
@@ -127,7 +192,6 @@ class PrivatePropertyTests {
         // Inner and Outer
         assertEquals(2, serializersByDescriptor.size)
 
-        val schemaDescriptor = output.schema.types.first().descriptor.name
         val c2 = DeserializationInput(factory).deserialize(output.obj)
 
         assertEquals(c1, c2)

node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt

@@ -20,6 +20,8 @@ import net.corda.nodeapi.internal.serialization.AllWhitelist
 import net.corda.nodeapi.internal.serialization.EmptyWhitelist
 import net.corda.nodeapi.internal.serialization.GeneratedAttachment
 import net.corda.nodeapi.internal.serialization.amqp.SerializerFactory.Companion.isPrimitive
+import net.corda.nodeapi.internal.serialization.amqp.custom.BigDecimalSerializer
+import net.corda.nodeapi.internal.serialization.amqp.custom.CurrencySerializer
 import net.corda.testing.contracts.DummyContract
 import net.corda.testing.core.BOB_NAME
 import net.corda.testing.core.SerializationEnvironmentRule
@@ -36,11 +38,13 @@ import org.junit.Test
 import java.io.ByteArrayInputStream
 import java.io.IOException
 import java.io.NotSerializableException
+import java.lang.reflect.Type
 import java.math.BigDecimal
 import java.nio.ByteBuffer
 import java.time.*
 import java.time.temporal.ChronoUnit
 import java.util.*
+import java.util.concurrent.ConcurrentHashMap
 import kotlin.reflect.full.superclasses
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
@@ -1080,4 +1084,38 @@ class SerializationOutputTests {
             serdes(obj, factory, factory2, expectedEqual = false, expectDeserializedEqual = false)
         }.isInstanceOf(MissingAttachmentsException::class.java)
     }
+
+    //
+    // Example stacktrace that this test is tryint to reproduce
+    //
+    // java.lang.IllegalArgumentException:
+    //      net.corda.core.contracts.TransactionState ->
+    //      data(net.corda.core.contracts.ContractState) ->
+    //      net.corda.finance.contracts.asset.Cash$State ->
+    //      amount(net.corda.core.contracts.Amount<net.corda.core.contracts.Issued<java.util.Currency>>) ->
+    //      net.corda.core.contracts.Amount<net.corda.core.contracts.Issued<java.util.Currency>> ->
+    //      displayTokenSize(java.math.BigDecimal) ->
+    //      wrong number of arguments
+    //
+    // So the actual problem was objects with multiple getters. The code wasn't looking for one with zero
+    // properties, just taking the first one it found with with the most applicable type, and the reflection
+    // ordering of the methods was random, thus occasionally we select the wrong one
+    //
+    @Test
+    fun reproduceWrongNumberOfArguments() {
+        val field = SerializerFactory::class.java.getDeclaredField("serializersByType").apply {
+            this.isAccessible = true
+        }
+
+        data class C(val a: Amount<Currency>)
+
+        val factory = testDefaultFactoryNoEvolution()
+        factory.register(net.corda.nodeapi.internal.serialization.amqp.custom.BigDecimalSerializer)
+        factory.register(net.corda.nodeapi.internal.serialization.amqp.custom.CurrencySerializer)
+
+        val c = C(Amount<Currency>(100, BigDecimal("1.5"), Currency.getInstance("USD")))
+
+        // were the issue not fixed we'd blow up here
+        SerializationOutput(factory).serialize(c)
+    }
 }