From 7894e723e88ca391cfef369013188959b2d00829 Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Tue, 22 Aug 2017 16:53:48 +0100 Subject: [PATCH] Fix evolver to work on nested schema types Dumb assumption in the initial implementation meant it could only evolve a top level type in the schema --- .../serialization/amqp/EvolutionSerializer.kt | 25 +++++++++++----- .../serialization/amqp/SerializerFactory.kt | 19 ++++++------ .../serialization/amqp/EvolvabilityTests.kt | 27 ++++++++++++++++++ .../amqp/EvolvabilityTests.changeSubType | Bin 0 -> 616 bytes 4 files changed, 53 insertions(+), 18 deletions(-) create mode 100644 node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.changeSubType 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 24c3667e09..6e0574567f 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 @@ -5,12 +5,11 @@ import org.apache.qpid.proton.codec.Data import java.lang.reflect.Type import java.io.NotSerializableException import kotlin.reflect.KFunction -import kotlin.reflect.full.primaryConstructor -import kotlin.reflect.full.findAnnotation import kotlin.reflect.jvm.javaType /** - * + * Serializer for deserialising objects whose definition has changed since they + * were serialised */ class EvolutionSerializer( clazz: Type, @@ -22,8 +21,13 @@ class EvolutionSerializer( override val propertySerializers: Collection = listOf() /** - * represents a paramter as would be passed to the constructor of the class as it was + * 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 idx where in the parameter list this parameter falls. required as the parameter + * order may have been changed and we need to know where into the list to look + * @param property object to read the actual property value */ data class oldParam (val type: Type, val idx: Int, val property: PropertySerializer) @@ -64,10 +68,10 @@ class EvolutionSerializer( * @param new is the Serializer built for the Class as it exists now, not * how it was serialised and persisted. */ - fun make (old: schemaAndDescriptor, new: ObjectSerializer, + fun make (old: CompositeType, new: ObjectSerializer, factory: SerializerFactory) : AMQPSerializer { - val oldFieldToType = (old.schema.types.first() as CompositeType).fields.map { + val oldFieldToType = old.fields.map { it.name as String? to it.getTypeAsClass(factory.classloader) as Type }.toMap() @@ -77,7 +81,7 @@ class EvolutionSerializer( val oldArgs = mutableMapOf() var idx = 0 - (old.schema.types.first() as CompositeType).fields.forEach { + old.fields.forEach { val returnType = it.getTypeAsClass(factory.classloader) oldArgs[it.name] = oldParam( returnType, idx++, PropertySerializer.make(it.name, null, returnType, factory)) @@ -91,6 +95,13 @@ class EvolutionSerializer( throw IllegalAccessException ("It should be impossible to write an evolution serializer") } + /** + * 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 + * constructor of the original state of the object, we need to map the new parameter order + * of the current constructor onto that list inserting nulls where new parameters are + * encountered + */ override fun readObject(obj: Any, schema: Schema, input: DeserializationInput): Any { if (obj !is List<*>) throw NotSerializableException("Body of described type is unexpected $obj") diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt index 6ae656171d..6d3f8dcbf9 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt @@ -30,9 +30,6 @@ data class schemaAndDescriptor (val schema: Schema, val typeDescriptor: Any) // TODO: profile for performance in general // TODO: use guava caches etc so not unbounded // TODO: do we need to support a transient annotation to exclude certain properties? -// TODO: incorporate the class carpenter for classes not on the classpath. -// TODO: apply class loader logic and an "app context" throughout this code. -// TODO: schema evolution solution when the fingerprints do not line up. // TODO: allow definition of well known types that are left out of the schema. // TODO: generally map Object to '*' all over the place in the schema and make sure use of '*' amd '?' is consistent and documented in generics. // TODO: found a document that states textual descriptors are Symbols. Adjust schema class appropriately. @@ -50,9 +47,9 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl : ClassLoader) { val classloader : ClassLoader get() = classCarpenter.classloader - fun getEvolutionSerializer(schema: schemaAndDescriptor, newSerializer: ObjectSerializer) : AMQPSerializer { - return serializersByDescriptor.computeIfAbsent(schema.typeDescriptor) { - EvolutionSerializer.make(schema, newSerializer, this) + fun getEvolutionSerializer(typeNotation: TypeNotation, newSerializer: ObjectSerializer) : AMQPSerializer { + return serializersByDescriptor.computeIfAbsent(typeNotation.descriptor.name!!) { + EvolutionSerializer.make(typeNotation as CompositeType, newSerializer, this) } } @@ -163,9 +160,9 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl : ClassLoader) { @Throws(NotSerializableException::class) fun get(typeDescriptor: Any, schema: Schema): AMQPSerializer { return serializersByDescriptor[typeDescriptor] ?: { - processSchema(schema) - serializersByDescriptor[typeDescriptor] ?: - throw NotSerializableException("Could not find type matching descriptor $typeDescriptor.") + processSchema(schemaAndDescriptor(schema, typeDescriptor)) + serializersByDescriptor[typeDescriptor] ?: throw NotSerializableException( + "Could not find type matching descriptor $typeDescriptor.") }() } @@ -196,8 +193,8 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl : ClassLoader) { // if we just successfully built a serialiser for the type but the type fingerprint // doesn't match that of the serialised object then we are dealing with an older // instance of the class, as such we need to build an evolverSerilaiser - if (serialiser.typeDescriptor != schema.typeDescriptor) { - getEvolutionSerializer(schema, serialiser as ObjectSerializer) + if (serialiser.typeDescriptor != typeNotation.descriptor.name) { + getEvolutionSerializer(typeNotation, serialiser as ObjectSerializer) } } catch (e: ClassNotFoundException) { if (sentinel || (typeNotation !is CompositeType)) throw e 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 29cb8169ba..9980603d7d 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 @@ -188,4 +188,31 @@ class EvolvabilityTests { assertEquals ("hello", deserializedCC.b) } + @Test + fun changeSubType() { + val sf = testDefaultFactory() + val path = EvolvabilityTests::class.java.getResource("EvolvabilityTests.changeSubType") + val f = File(path.toURI()) + val oa = 100 + val ia = 200 + + // Original version of the class as it was serialised + // + // data class Inner (val a: Int) + // data class Outer (val a: Int, val b: Inner) + // val scc = SerializationOutput(sf).serialize(Outer(oa, Inner (ia))) + // f.writeBytes(scc.bytes) + // println ("Path = $path") + + // Add a parameter to inner but keep outer unchanged + data class Inner (val a: Int, val b: String?) + data class Outer (val a: Int, val b: Inner) + + val sc2 = f.readBytes() + val outer = DeserializationInput(sf).deserialize(SerializedBytes(sc2)) + + assertEquals (oa, outer.a) + assertEquals (ia, outer.b.a) + assertEquals (null, outer.b.b) + } } \ No newline at end of file diff --git a/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.changeSubType b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.changeSubType new file mode 100644 index 0000000000000000000000000000000000000000..bcc0475b4a883a92cb2be6a2b3d78fda1de825b9 GIT binary patch literal 616 zcmYe!FG@*dWME)u0Ahv%w-^{NFfcF$0@+Lq3zhOxOZ1XKDy*U`5-XD`4Fl536N@aI zOAQNsN=+;sZEX)2FomR`sC0J=EH|$33-+q;&hyQR49)P%bV>xNWMeD@8hZk66BE!T z#^*pGkd=)0z*aT@Nsud8K|*3cHtWJ*xFhuP@>5b13o`XG^GZ^S@)C3Oic^a+6LT`F z5=%1k^YjvP3k&pI%kp!|5|c7>GD|8$Qj1H9RgyCj^U_m;OOrw>3sP15OM!YFT;Z-@ zKA^@#j7J>~v@^jqu^bR!U&xrakQwL_*M*D*4vtPpa&bVpBx<|cGcPZ-$khSaCe{Nh YsUAFFYw!dQuGn=rAj^mp1Q@{s07t{gj{pDw literal 0 HcmV?d00001