More Review Comments

Move more logic into the constructor factory for the evolver seraliser
rather than everytime we read
This commit is contained in:
Katelyn Baker
2017-08-24 15:49:59 +01:00
parent cf9fbc715d
commit e4a2529de0
3 changed files with 29 additions and 33 deletions

View File

@ -1,6 +1,6 @@
package net.corda.nodeapi.internal.serialization.amqp
import net.corda.core.serialization.EvolvedSerializerConstructor
import net.corda.core.serialization.DeprecatedConstructorForDeserialization
import net.corda.nodeapi.internal.serialization.carpenter.getTypeAsClass
import org.apache.qpid.proton.codec.Data
import java.lang.reflect.Type
@ -16,9 +16,8 @@ import kotlin.reflect.jvm.javaType
class EvolutionSerializer(
clazz: Type,
factory: SerializerFactory,
val oldParams : Map<String, OldParam>,
val newParams : Map<String, Boolean>,
override val kotlinConstructor: KFunction<Any>?) : ObjectSerializer (clazz, factory) {
val readers: List<OldParam?>,
override val kotlinConstructor: KFunction<Any>?) : ObjectSerializer(clazz, factory) {
// explicitly null this out as we won't be using this list
override val propertySerializers: Collection<PropertySerializer> = listOf()
@ -32,7 +31,7 @@ class EvolutionSerializer(
* 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) {
data class OldParam(val type: Type, val idx: Int, val property: PropertySerializer) {
fun readProperty(paramValues: List<*>, schema: Schema, input: DeserializationInput) =
property.readProperty(paramValues[idx], schema, input)
}
@ -52,12 +51,12 @@ class EvolutionSerializer(
val clazz: Class<*> = type.asClass()!!
if (!isConcrete(clazz)) return null
val oldArgumentSet = oldArgs.map { Pair (it.key, it.value) }
val oldArgumentSet = oldArgs.map { Pair(it.key, it.value) }
var maxConstructorVersion = Integer.MIN_VALUE
var constructor: KFunction<Any>? = null
clazz.kotlin.constructors.forEach {
val version = it.findAnnotation<EvolvedSerializerConstructor>()?.version ?: Integer.MIN_VALUE
val version = it.findAnnotation<DeprecatedConstructorForDeserialization>()?.version ?: Integer.MIN_VALUE
if (oldArgumentSet.containsAll(it.parameters.map { v -> Pair(v.name, v.type.javaType) }) &&
version > maxConstructorVersion) {
constructor = it
@ -79,8 +78,8 @@ 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: CompositeType, new: ObjectSerializer,
factory: SerializerFactory) : AMQPSerializer<Any> {
fun make(old: CompositeType, new: ObjectSerializer,
factory: SerializerFactory): AMQPSerializer<Any> {
val oldFieldToType = old.fields.map {
it.name as String? to it.getTypeAsClass(factory.classloader) as Type
@ -98,14 +97,19 @@ class EvolutionSerializer(
returnType, idx++, PropertySerializer.make(it.name, null, returnType, factory))
}
val newArgs = constructor.parameters.associateBy({ it.name!! }, {it.type.isMarkedNullable})
val readers = constructor.parameters.map {
oldArgs[it.name!!] ?: if (!it.type.isMarkedNullable) {
throw NotSerializableException(
"New parameter ${it.name} is mandatory, should be nullable for evolution to worK")
} else null
}
return EvolutionSerializer(new.type, factory, oldArgs, newArgs, constructor)
return EvolutionSerializer(new.type, factory, readers, constructor)
}
}
override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) {
throw IllegalAccessException ("It should be impossible to write an evolution serializer")
throw IllegalAccessException("It should be impossible to write an evolution serializer")
}
/**
@ -118,15 +122,7 @@ class EvolutionSerializer(
override fun readObject(obj: Any, schema: Schema, input: DeserializationInput): Any {
if (obj !is List<*>) throw NotSerializableException("Body of described type is unexpected $obj")
return construct(newParams.map {
val param = oldParams[it.key]
if (param == null && !it.value) {
throw NotSerializableException(
"New parameter ${it.key} is mandatory, should be nullable for evolution to worK")
}
param?.readProperty(obj, schema, input)
})
return construct(readers.map { it?.readProperty(obj, schema, input) })
}
}

View File

@ -1,7 +1,7 @@
package net.corda.nodeapi.internal.serialization.amqp
import net.corda.core.serialization.SerializedBytes
import net.corda.core.serialization.EvolvedSerializerConstructor
import net.corda.core.serialization.DeprecatedConstructorForDeserialization
import org.junit.Test
import java.io.File
@ -187,7 +187,7 @@ class EvolvabilityTests {
@Suppress("UNUSED")
data class CC (val a: Int, val b: String) {
@EvolvedSerializerConstructor(1)
@DeprecatedConstructorForDeserialization(1)
constructor (a: Int) : this (a, "hello")
}
@ -246,7 +246,7 @@ class EvolvabilityTests {
data class CC (val a: Int, val b: Int, val c: String, val d: String) {
// ensure none of the original parameters align with the initial
// construction order
@EvolvedSerializerConstructor(1)
@DeprecatedConstructorForDeserialization(1)
constructor (c: String, a: Int, b: Int) : this (a, b, c, "wibble")
}
@ -282,7 +282,7 @@ class EvolvabilityTests {
// ensure none of the original parameters align with the initial
// construction order
@Suppress("UNUSED")
@EvolvedSerializerConstructor(1)
@DeprecatedConstructorForDeserialization(1)
constructor (c: String, a: Int) : this (a, c, "wibble")
}
@ -324,11 +324,11 @@ class EvolvabilityTests {
@Suppress("UNUSED")
data class C (val e: Int, val c: Int, val b: Int, val a: Int, val d: Int) {
@EvolvedSerializerConstructor(1)
@DeprecatedConstructorForDeserialization(1)
constructor (b: Int, a: Int) : this (-1, -1, b, a, -1)
@EvolvedSerializerConstructor(2)
@DeprecatedConstructorForDeserialization(2)
constructor (a: Int, c: Int, b: Int) : this (-1, c, b, a, -1)
@EvolvedSerializerConstructor(3)
@DeprecatedConstructorForDeserialization(3)
constructor (a: Int, b: Int, c: Int, d: Int) : this (-1, c, b, a, d)
}
@ -418,13 +418,13 @@ class EvolvabilityTests {
@Suppress("UNUSED")
data class C (val b: Int, val c: Int, val d: Int, val e: Int, val f: Int, val g: Int) {
@EvolvedSerializerConstructor(1)
@DeprecatedConstructorForDeserialization(1)
constructor (b: Int, c: Int) : this (b, c, -1, -1, -1, -1)
@EvolvedSerializerConstructor(2)
@DeprecatedConstructorForDeserialization(2)
constructor (b: Int, c: Int, d: Int) : this (b, c, d, -1, -1, -1)
@EvolvedSerializerConstructor(3)
@DeprecatedConstructorForDeserialization(3)
constructor (b: Int, c: Int, d: Int, e: Int) : this (b, c, d, e, -1, -1)
@EvolvedSerializerConstructor(4)
@DeprecatedConstructorForDeserialization(4)
constructor (b: Int, c: Int, d: Int, e: Int, f: Int) : this (b, c, d, e, f, -1)
}