Add constructor annotation for evolve selection

Current implementation is such that if we find *a* constructor that
works for us we use that, this is of course rather non deterministic, it
also means we may not select the best constructor

for example

old versions had constructors that took

V1: A B
V2: A B C D

if current version, V3 is

V3: A B C D E

which provides secondary constructors for the above then there is a
chance we'd de-serialise objects that were serialised as V2 using the V1
constructor and thus throw away information we don't need to

Additional Changes:

Fixes following rebase onto master

core/src/main/kotlin/net/corda/core/serialization/CordaSerializerConstructor.kt

@@ -0,0 +1,11 @@
+package net.corda.core.serialization
+
+/**
+ * This annotation is a marker to indicate which secondary constructors shuold be considered, and in which
+ * order, for evolving objects during their deserialisation.
+ */
+@Target(AnnotationTarget.CONSTRUCTOR)
+@Retention(AnnotationRetention.RUNTIME)
+annotation class CordaSerializerConstructor(val version: Int)
+
+

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

@@ -1,10 +1,12 @@
 package net.corda.nodeapi.internal.serialization.amqp
 
+import net.corda.core.serialization.CordaSerializerConstructor
 import net.corda.nodeapi.internal.serialization.carpenter.getTypeAsClass
 import org.apache.qpid.proton.codec.Data
 import java.lang.reflect.Type
 import java.io.NotSerializableException
 import kotlin.reflect.KFunction
+import kotlin.reflect.full.findAnnotation
 import kotlin.reflect.jvm.javaType
 
 /**
@@ -48,15 +50,20 @@ class EvolutionSerializer(
 
             val oldArgumentSet = oldArgs.map { Pair (it.key, it.value) }
 
+            var maxConstructorVersion = Integer.MIN_VALUE
+            var constructor: KFunction<Any>? = null
             clazz.kotlin.constructors.forEach {
-                if (oldArgumentSet.containsAll(it.parameters.map { v -> Pair(v.name, v.type.javaType) })) {
-                    return it
+                val version = it.findAnnotation<CordaSerializerConstructor>()?.version ?: Integer.MIN_VALUE
+                if (oldArgumentSet.containsAll(it.parameters.map { v -> Pair(v.name, v.type.javaType) }) &&
+                        version > maxConstructorVersion) {
+                    constructor = it
+                    maxConstructorVersion = version
                 }
             }
 
             // 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 constructorForDeserialization(type)
+            return constructor ?: constructorForDeserialization(type)
         }
 
         /**

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

@@ -53,8 +53,8 @@ sealed class PropertySerializer(val name: String, val readMethod: Method?, val r
     }
 
     companion object {
-        fun make(name: String, readMethod: Method, resolvedType: Type, factory: SerializerFactory): PropertySerializer {
-            readMethod.isAccessible = true
+        fun make(name: String, readMethod: Method?, resolvedType: Type, factory: SerializerFactory): PropertySerializer {
+            readMethod?.isAccessible = true
             if (SerializerFactory.isPrimitive(resolvedType)) {
                 return when(resolvedType) {
                     Char::class.java, Character::class.java -> AMQPCharPropertySerializer(name, readMethod)

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

@@ -1,6 +1,8 @@
 package net.corda.nodeapi.internal.serialization.amqp
 
 import net.corda.core.serialization.SerializedBytes
+import net.corda.core.serialization.CordaSerializerConstructor
+
 import org.junit.Test
 import java.io.File
 import java.io.NotSerializableException
@@ -178,6 +180,7 @@ class EvolvabilityTests {
 
         @Suppress("UNUSED")
         data class CC (val a: Int, val b: String) {
+            @CordaSerializerConstructor(1)
             constructor (a: Int) : this (a, "hello")
         }
 
@@ -188,6 +191,168 @@ class EvolvabilityTests {
         assertEquals ("hello", deserializedCC.b)
     }
 
+    @Test(expected = NotSerializableException::class)
+    @Suppress("UNUSED")
+    fun addMandatoryFieldWithAltConstructorUnAnnotated() {
+        val sf = testDefaultFactory()
+        val path = EvolvabilityTests::class.java.getResource(
+                "EvolvabilityTests.addMandatoryFieldWithAltConstructorUnAnnotated")
+        val f = File(path.toURI())
+        @Suppress("UNUSED_VARIABLE")
+        val A = 1
+
+        // Original version of the class as it was serialised
+        //
+        // data class CC(val a: Int)
+        // val scc = SerializationOutput(sf).serialize(CC(A))
+        // f.writeBytes(scc.bytes)
+        // println ("Path = $path")
+
+        data class CC (val a: Int, val b: String) {
+            // constructor annotation purposefully omitted
+            constructor (a: Int) : this (a, "hello")
+        }
+
+        // we expect this to throw as we should not find any constructors
+        // capable of dealing with this
+        DeserializationInput(sf).deserialize(SerializedBytes<CC>(f.readBytes()))
+    }
+
+    @Test
+    fun addMandatoryFieldWithAltReorderedConstructor() {
+        val sf = testDefaultFactory()
+        val path = EvolvabilityTests::class.java.getResource(
+                "EvolvabilityTests.addMandatoryFieldWithAltReorderedConstructor")
+        val f = File(path.toURI())
+        val A = 1
+        val B = 100
+        val C = "This is not a banana"
+
+        // Original version of the class as it was serialised
+        //
+        // data class CC(val a: Int, val b: Int, val c: String)
+        // val scc = SerializationOutput(sf).serialize(CC(A, B, C))
+        // f.writeBytes(scc.bytes)
+        // println ("Path = $path")
+
+        @Suppress("UNUSED")
+        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
+            @CordaSerializerConstructor(1)
+            constructor (c: String, a: Int, b: Int) : this (a, b, c, "wibble")
+        }
+
+        val sc2 = f.readBytes()
+        val deserializedCC = DeserializationInput(sf).deserialize(SerializedBytes<CC>(sc2))
+
+        assertEquals (A, deserializedCC.a)
+        assertEquals (B, deserializedCC.b)
+        assertEquals (C, deserializedCC.c)
+        assertEquals ("wibble", deserializedCC.d)
+    }
+
+    @Test
+    fun addMandatoryFieldWithAltReorderedConstructorAndRemoval() {
+        val sf = testDefaultFactory()
+        val path = EvolvabilityTests::class.java.getResource(
+                "EvolvabilityTests.addMandatoryFieldWithAltReorderedConstructorAndRemoval")
+        val f = File(path.toURI())
+        val A = 1
+        @Suppress("UNUSED_VARIABLE")
+        val B = 100
+        val C = "This is not a banana"
+
+        // Original version of the class as it was serialised
+        //
+        // data class CC(val a: Int, val b: Int, val c: String)
+        // val scc = SerializationOutput(sf).serialize(CC(A, B, C))
+        // f.writeBytes(scc.bytes)
+        // println ("Path = $path")
+
+        // b is removed, d is added
+        data class CC (val a: Int, val c: String, val d: String) {
+            // ensure none of the original parameters align with the initial
+            // construction order
+            @Suppress("UNUSED")
+            @CordaSerializerConstructor(1)
+            constructor (c: String, a: Int) : this (a, c, "wibble")
+        }
+
+        val sc2 = f.readBytes()
+        val deserializedCC = DeserializationInput(sf).deserialize(SerializedBytes<CC>(sc2))
+
+        assertEquals (A, deserializedCC.a)
+        assertEquals (C, deserializedCC.c)
+        assertEquals ("wibble", deserializedCC.d)
+    }
+
+    @Test
+    fun multiVersion() {
+        val sf = testDefaultFactory()
+        val path1 = EvolvabilityTests::class.java.getResource("EvolvabilityTests.multiVersion.1")
+        val path2 = EvolvabilityTests::class.java.getResource("EvolvabilityTests.multiVersion.2")
+        val path3 = EvolvabilityTests::class.java.getResource("EvolvabilityTests.multiVersion.3")
+
+        @Suppress("UNUSED_VARIABLE")
+        val f = File(path1.toURI())
+
+        val a = 100
+        val b = 200
+        val c = 300
+        val d = 400
+
+        // Original version of the class as it was serialised
+        //
+        // Version 1:
+        // data class C (val a: Int, val b: Int)
+        // Version 2 - add param c
+        // data class C (val c: Int, val b: Int, val a: Int)
+        // Version 3 - add param d
+        // data class C (val b: Int, val c: Int, val d: Int, val a: Int)
+        //
+        // val scc = SerializationOutput(sf).serialize(C(b, c, d, a))
+        // f.writeBytes(scc.bytes)
+        // println ("Path = $path1")
+
+        @Suppress("UNUSED")
+        data class C (val e: Int, val c: Int, val b: Int, val a: Int, val d: Int) {
+            @CordaSerializerConstructor(1)
+            constructor (b: Int, a: Int) : this (-1, -1, b, a, -1)
+            @CordaSerializerConstructor(2)
+            constructor (a: Int, c: Int, b: Int) : this (-1, c, b, a, -1)
+            @CordaSerializerConstructor(3)
+            constructor (a: Int, b: Int, c: Int, d: Int) : this (-1, c, b, a, d)
+        }
+
+        val sb1 = File(path1.toURI()).readBytes()
+        val db1 = DeserializationInput(sf).deserialize(SerializedBytes<C>(sb1))
+
+        assertEquals(a, db1.a)
+        assertEquals(b, db1.b)
+        assertEquals(-1, db1.c)
+        assertEquals(-1, db1.d)
+        assertEquals(-1, db1.e)
+
+        val sb2 = File(path2.toURI()).readBytes()
+        val db2 = DeserializationInput(sf).deserialize(SerializedBytes<C>(sb2))
+
+        assertEquals(a, db2.a)
+        assertEquals(b, db2.b)
+        assertEquals(c, db2.c)
+        assertEquals(-1, db2.d)
+        assertEquals(-1, db2.e)
+
+        val sb3 = File(path3.toURI()).readBytes()
+        val db3 = DeserializationInput(sf).deserialize(SerializedBytes<C>(sb3))
+
+        assertEquals(a, db3.a)
+        assertEquals(b, db3.b)
+        assertEquals(c, db3.c)
+        assertEquals(d, db3.d)
+        assertEquals(-1, db3.e)
+    }
+
     @Test
     fun changeSubType() {
         val sf = testDefaultFactory()
@@ -215,4 +380,75 @@ class EvolvabilityTests {
         assertEquals (ia, outer.b.a)
         assertEquals (null, outer.b.b)
     }
+
+    @Test
+    fun multiVersionWithRemoval() {
+        val sf = testDefaultFactory()
+        val path1 = EvolvabilityTests::class.java.getResource("EvolvabilityTests.multiVersionWithRemoval.1")
+        val path2 = EvolvabilityTests::class.java.getResource("EvolvabilityTests.multiVersionWithRemoval.2")
+        val path3 = EvolvabilityTests::class.java.getResource("EvolvabilityTests.multiVersionWithRemoval.3")
+
+        @Suppress("UNUSED_VARIABLE")
+        val a = 100
+        val b = 200
+        val c = 300
+        val d = 400
+        val e = 500
+        val f = 600
+
+        // Original version of the class as it was serialised
+        //
+        // Version 1:
+        // data class C (val a: Int, val b: Int, val c: Int)
+        // Version 2 - add param c
+        // data class C (val b: Int, val c: Int, val d: Int, val e: Int)
+        // Version 3 - add param d
+        // data class C (val b: Int, val c: Int, val d: Int, val e: Int, val f: Int)
+        //
+        // val scc = SerializationOutput(sf).serialize(C(b, c, d, e, f))
+        // File(path1.toURI()).writeBytes(scc.bytes)
+        // println ("Path = $path1")
+
+        @Suppress("UNUSED")
+        data class C (val b: Int, val c: Int, val d: Int, val e: Int, val f: Int, val g: Int) {
+            @CordaSerializerConstructor(1)
+            constructor (b: Int, c: Int) : this (b, c, -1, -1, -1, -1)
+            @CordaSerializerConstructor(2)
+            constructor (b: Int, c: Int, d: Int) : this (b, c, d, -1, -1, -1)
+            @CordaSerializerConstructor(3)
+            constructor (b: Int, c: Int, d: Int, e: Int) : this (b, c, d, e, -1, -1)
+            @CordaSerializerConstructor(4)
+            constructor (b: Int, c: Int, d: Int, e: Int, f: Int) : this (b, c, d, e, f, -1)
+        }
+
+        val sb1 = File(path1.toURI()).readBytes()
+        val db1 = DeserializationInput(sf).deserialize(SerializedBytes<C>(sb1))
+
+        assertEquals(b, db1.b)
+        assertEquals(c, db1.c)
+        assertEquals(-1, db1.d) // must not be set by calling constructor 2 by mistake
+        assertEquals(-1, db1.e)
+        assertEquals(-1, db1.f)
+        assertEquals(-1, db1.g)
+
+        val sb2 = File(path2.toURI()).readBytes()
+        val db2 = DeserializationInput(sf).deserialize(SerializedBytes<C>(sb2))
+
+        assertEquals(b, db2.b)
+        assertEquals(c, db2.c)
+        assertEquals(d, db2.d)
+        assertEquals(e, db2.e)
+        assertEquals(-1, db2.f)
+        assertEquals(-1, db1.g)
+
+        val sb3 = File(path3.toURI()).readBytes()
+        val db3 = DeserializationInput(sf).deserialize(SerializedBytes<C>(sb3))
+
+        assertEquals(b, db3.b)
+        assertEquals(c, db3.c)
+        assertEquals(d, db3.d)
+        assertEquals(e, db3.e)
+        assertEquals(f, db3.f)
+        assertEquals(-1, db3.g)
+    }
 }