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 b2dfa16f53..0b6b517167 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 @@ -2,6 +2,8 @@ package net.corda.nodeapi.internal.serialization.amqp import net.corda.core.serialization.DeprecatedConstructorForDeserialization import net.corda.nodeapi.internal.serialization.carpenter.getTypeAsClass +import net.corda.core.utilities.contextLogger +import net.corda.core.utilities.debug import org.apache.qpid.proton.codec.Data import java.lang.reflect.Type import java.io.NotSerializableException @@ -16,10 +18,8 @@ import kotlin.reflect.jvm.javaType * * @property oldReaders A linked map representing the properties of the object as they were serialized. Note * this may contain properties that are no longer needed by the class. These *must* be read however to ensure - * any refferenced objects in the object stream are captured properly - * @property kotlinConstructor - * @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 + * any referenced objects in the object stream are captured properly + * @property kotlinConstructor reference to the constructor used to instantiate an instance of the class. */ abstract class EvolutionSerializer( clazz: Type, @@ -39,15 +39,21 @@ abstract class EvolutionSerializer( * @param property object to read the actual property value */ 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) { - new[resultsIndex] = this - } - } + fun readProperty(obj: Any?, schemas: SerializationSchemas, input: DeserializationInput, + new: Array + ) = property.readProperty(obj, schemas, input).apply { + if(resultsIndex >= 0) { + new[resultsIndex] = this + } + } + override fun toString(): String { + return "resultsIndex = $resultsIndex property = ${property.name}" + } } companion object { + val logger = contextLogger() + /** * Unlike the generic deserialization case where we need to locate the primary constructor * for the object (or our best guess) in the case of an object whose structure has changed @@ -63,22 +69,37 @@ abstract class EvolutionSerializer( if (!isConcrete(clazz)) return null - val oldArgumentSet = oldArgs.map { Pair(it.key as String?, it.value.property.resolvedType) } - + val oldArgumentSet = oldArgs.map { Pair(it.key as String?, it.value.property.resolvedType.asClass()) } var maxConstructorVersion = Integer.MIN_VALUE var constructor: KFunction? = null + clazz.kotlin.constructors.forEach { val version = it.findAnnotation()?.version ?: Integer.MIN_VALUE - if (oldArgumentSet.containsAll(it.parameters.map { v -> Pair(v.name, v.type.javaType) }) && - version > maxConstructorVersion) { + + if (version > maxConstructorVersion && + oldArgumentSet.containsAll(it.parameters.map { v -> Pair(v.name, v.type.javaType.asClass()) }) + ) { constructor = it maxConstructorVersion = version + + with(logger) { + info("Select annotated constructor version=$version nparams=${it.parameters.size}") + debug{" params=${it.parameters}"} + } + } else if (version != Integer.MIN_VALUE){ + with(logger) { + info("Ignore annotated constructor version=$version nparams=${it.parameters.size}") + debug{" params=${it.parameters}"} + } } } // if we didn't get an exact match revert to existing behaviour, if the new parameters // are not mandatory (i.e. nullable) things are fine - return constructor ?: constructorForDeserialization(type) + return constructor ?: run { + logger.info("Failed to find annotated historic constructor") + constructorForDeserialization(type) + } } private fun makeWithConstructor( 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 5eb6bbbab7..d4b6573c1a 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 @@ -18,6 +18,7 @@ import java.io.NotSerializableException import java.net.URI import java.time.Instant import kotlin.test.assertEquals +import net.corda.nodeapi.internal.serialization.amqp.custom.InstantSerializer // To regenerate any of the binary test files do the following // @@ -38,8 +39,8 @@ class EvolvabilityTests { val sf = testDefaultFactory() val resource = "EvolvabilityTests.simpleOrderSwapSameType" - val A = 1 - val B = 2 + val a = 1 + val b = 2 // Original version of the class for the serialised version of this class // data class C (val a: Int, val b: Int) @@ -54,8 +55,8 @@ class EvolvabilityTests { val sc2 = f.readBytes() val deserializedC = DeserializationInput(sf).deserialize(SerializedBytes(sc2)) - assertEquals(A, deserializedC.a) - assertEquals(B, deserializedC.b) + assertEquals(a, deserializedC.a) + assertEquals(b, deserializedC.b) } @Test @@ -206,6 +207,86 @@ class EvolvabilityTests { assertEquals("hello", deserializedCC.b) } + @Test + fun addMandatoryFieldWithAltConstructorForceReorder() { + val sf = testDefaultFactory() + val z = 30 + val y = 20 + val resource = "EvolvabilityTests.addMandatoryFieldWithAltConstructorForceReorder" + + // Original version of the class as it was serialised + // data class CC(val z: Int, val y: Int) + // File(URI("$localPath/$resource")).writeBytes(SerializationOutput(sf).serialize(CC(z, y)).bytes) + + @Suppress("UNUSED") + data class CC(val z: Int, val y: Int, val a: String) { + @DeprecatedConstructorForDeserialization(1) + constructor (z: Int, y: Int) : this(z, y, "10") + } + + val url = EvolvabilityTests::class.java.getResource(resource) + val deserializedCC = DeserializationInput(sf).deserialize(SerializedBytes(url.readBytes())) + + assertEquals("10", deserializedCC.a) + assertEquals(y, deserializedCC.y) + assertEquals(z, deserializedCC.z) + } + + @Test + fun moreComplexNonNullWithReorder() { + val resource = "${javaClass.simpleName}.${testName()}" + + data class NetworkParametersExample( + val minimumPlatformVersion: Int, + val notaries: List, + val maxMessageSize: Int, + val maxTransactionSize: Int, + val modifiedTime: Instant, + val epoch: Int, + val whitelistedContractImplementations: Map>, + /* to regenerate test class, comment out this element */ + val eventHorizon: Int + ) { + // when regenerating test class this won't be required + @DeprecatedConstructorForDeserialization(1) + @Suppress("UNUSED") + constructor ( + minimumPlatformVersion: Int, + notaries: List, + maxMessageSize: Int, + maxTransactionSize: Int, + modifiedTime: Instant, + epoch: Int, + whitelistedContractImplementations: Map> + ) : this(minimumPlatformVersion, + notaries, + maxMessageSize, + maxTransactionSize, + modifiedTime, + epoch, + whitelistedContractImplementations, + Int.MAX_VALUE) + } + + val factory = testDefaultFactory().apply { + register(InstantSerializer(this)) + } + + // Uncomment to regenerate test case + // File(URI("$localPath/$resource")).writeBytes(SerializationOutput(factory).serialize( + // NetworkParametersExample( + // 10, + // listOf("Notary1", "Notary2"), + // 100, + // 10, + // Instant.now(), + // 9, + // mapOf("A" to listOf(1, 2, 3), "B" to listOf (4, 5, 6)))).bytes) + + val url = EvolvabilityTests::class.java.getResource(resource) + DeserializationInput(factory).deserialize(SerializedBytes(url.readBytes())) + } + @Test(expected = NotSerializableException::class) @Suppress("UNUSED") fun addMandatoryFieldWithAltConstructorUnAnnotated() { @@ -487,7 +568,7 @@ class EvolvabilityTests { // @Test @Ignore("Test fails after moving NetworkParameters and NotaryInfo into core from node-api") - fun readBrokenNetworkParameters(){ + 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) @@ -524,7 +605,7 @@ class EvolvabilityTests { val resource = "networkParams.." val DUMMY_NOTARY = TestIdentity(DUMMY_NOTARY_NAME, 20).party val networkParameters = NetworkParameters( - 3, listOf(NotaryInfo(DUMMY_NOTARY, false)),1000, 1000, Instant.EPOCH, 1 , emptyMap()) + 3, listOf(NotaryInfo(DUMMY_NOTARY, false)), 1000, 1000, Instant.EPOCH, 1, emptyMap()) val sf = testDefaultFactory() sf.register(net.corda.nodeapi.internal.serialization.amqp.custom.InstantSerializer(sf)) diff --git a/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.addMandatoryFieldWithAltConstructorForceReorder b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.addMandatoryFieldWithAltConstructorForceReorder new file mode 100644 index 0000000000..b4ead2ac82 Binary files /dev/null and b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.addMandatoryFieldWithAltConstructorForceReorder differ diff --git a/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.moreComplexNonNullWithReorder b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.moreComplexNonNullWithReorder new file mode 100644 index 0000000000..df1d294642 Binary files /dev/null and b/node-api/src/test/resources/net/corda/nodeapi/internal/serialization/amqp/EvolvabilityTests.moreComplexNonNullWithReorder differ