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 78ca924deb..745c5adf2a 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 @@ -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, - override val kotlinConstructor: KFunction?, - private val constructorArgs: Array) : ObjectSerializer(clazz, factory) { + protected val oldReaders: Map, + override val kotlinConstructor: KFunction?) : 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) = property.readProperty(obj, schemas, input).apply { if(resultsIndex >= 0) { @@ -64,7 +62,7 @@ class EvolutionSerializer( 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? = null @@ -82,6 +80,36 @@ class EvolutionSerializer( return constructor ?: constructorForDeserialization(type) } + private fun makeWithConstructor( + new: ObjectSerializer, + factory: SerializerFactory, + constructor: KFunction, + readersAsSerialized: Map): AMQPSerializer { + val constructorArgs = arrayOfNulls(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, + readersAsSerialized: Map, + classProperties: Map): AMQPSerializer { + 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. @@ -96,45 +124,43 @@ class EvolutionSerializer( fun make(old: CompositeType, new: ObjectSerializer, factory: SerializerFactory): AMQPSerializer { - val readersAsSerialized = linkedMapOf( - *(old.fields.map { - val returnType = try { - it.getTypeAsClass(factory.classloader) - } catch (e: ClassNotFoundException) { - throw NotSerializableException(e.message) - } - - it.name to OldParam( - returnType, - -1, - PropertySerializer.make( - it.name, PublicPropertyReader(null), returnType, factory)) - }.toTypedArray()) - ) + // The order in which the properties were serialised is important and must be preserved + val readersAsSerialized = LinkedHashMap() + old.fields.forEach { + readersAsSerialized[it.name] = try { + OldParam(-1, PropertySerializer.make(it.name, EvolutionPropertyReader(), + it.getTypeAsClass(factory.classloader), factory)) + } catch (e: ClassNotFoundException) { + throw NotSerializableException(e.message) + } + } val constructor = getEvolverConstructor(new.type, readersAsSerialized) ?: throw NotSerializableException( "Attempt to deserialize an interface: ${new.type}. Serialized form is invalid.") - val constructorArgs = arrayOfNulls(constructor.parameters.size) + val classProperties = new.type.asClass()?.propertyDescriptors() ?: emptyMap() - 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 if (classProperties.isNotEmpty() && constructor.parameters.isEmpty()) { + makeWithSetters(new, factory, constructor, readersAsSerialized, classProperties) + } + else { + makeWithConstructor(new, factory, constructor, readersAsSerialized) } - - return EvolutionSerializer(new.type, factory, readersAsSerialized, constructor, constructorArgs) } } - 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, + kotlinConstructor: KFunction?, + private val constructorArgs: Array) : 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, + kotlinConstructor: KFunction?, + private val setters: Map) : 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 } } 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 537912a044..b2cfcc9311 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 @@ -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().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. * 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 b4c15a0110..b10344353f 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 @@ -253,7 +253,7 @@ internal fun 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, type: Type, factory: SerializerFactory): List { diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt index b18abd9f82..0fce099955 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.kt @@ -538,4 +538,45 @@ class EvolvabilityTests { File(URI("$localPath/$resource")).writeBytes( signedAndSerialized.bytes) } -} \ No newline at end of file + @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(sc2)) + + assertEquals(1, deserializedC.a) + assertEquals(2, deserializedC.b) + assertEquals(4, deserializedC.d) + assertEquals(5, deserializedC.e) + } +} diff --git a/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.getterSetterEvolver1 b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.getterSetterEvolver1 new file mode 100644 index 0000000000..c544432e6d Binary files /dev/null and b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.getterSetterEvolver1 differ