CORDA-1662 - Corda Serialization Evolution breaksdown with Java classes (#3427)

Nullability logic was relying on annotations that Kotlin applies by
default but is left to the developer in Javaland. Change this around
so it works for both.

In Kotlin, the property must be nullable, in Java, it can't be a
primitive.

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

@@ -10,6 +10,7 @@ import java.io.NotSerializableException
 import kotlin.reflect.KFunction
 import kotlin.reflect.full.findAnnotation
 import kotlin.reflect.jvm.javaType
+import kotlin.reflect.jvm.jvmErasure
 
 
 /**
@@ -109,12 +110,29 @@ abstract class EvolutionSerializer(
                 readersAsSerialized: Map<String, OldParam>): AMQPSerializer<Any> {
             val constructorArgs = arrayOfNulls<Any?>(constructor.parameters.size)
 
+            // Java doesn't care about nullability unless it's a primitive in which
+            // case it can't be referenced. Unfortunately whilst Kotlin does apply
+            // Nullability annotations we cannot use them here as they aren't
+            // retained at runtime so we cannot rely on the absence of
+            // any particular NonNullable annotation type to indicate cross
+            // compiler nullability
+            val isKotlin = (new.type.javaClass.declaredAnnotations.any {
+                        it.annotationClass.qualifiedName == "kotlin.Metadata"
+            })
+
             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")
+                if ((readersAsSerialized[it.value.name!!] ?.apply { this.resultsIndex = it.index }) == null) {
+                    // If there is no value in the byte stream to map to the parameter of the constructor
+                    // this is ok IFF it's a Kotlin class and the parameter is non nullable OR
+                    // its a Java class and the parameter is anything but an unboxed primitive.
+                    // Otherwise we throw the error and leave
+                    if ((isKotlin && !it.value.type.isMarkedNullable)
+                            || (!isKotlin && isJavaPrimitive(it.value.type.jvmErasure.java))
+                    ) {
+                        throw NotSerializableException(
+                                "New parameter \"${it.value.name}\" is mandatory, should be nullable for evolution " +
+                                        "to work, isKotlinClass=$isKotlin type=${it.value.type}")
+                    }
                 }
             }
             return EvolutionSerializerViaConstructor (new.type, factory, readersAsSerialized, constructor, constructorArgs)
@@ -143,8 +161,10 @@ abstract class EvolutionSerializer(
          * @param factory the [SerializerFactory] associated with the serialization
          * context this serializer is being built for
          */
-        fun make(old: CompositeType, new: ObjectSerializer,
-                 factory: SerializerFactory): AMQPSerializer<Any> {
+        fun make(old: CompositeType,
+                 new: ObjectSerializer,
+                 factory: SerializerFactory
+        ): AMQPSerializer<Any> {
             // The order in which the properties were serialised is important and must be preserved
             val readersAsSerialized = LinkedHashMap<String, OldParam>()
             old.fields.forEach {

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

@@ -570,3 +570,18 @@ fun Class<*>.objectInstance() =
                 }
             }
         }
+
+fun isJavaPrimitive(type: Class<*>) = type in JavaPrimitiveTypes.primativeTypes
+
+private object JavaPrimitiveTypes {
+    val primativeTypes = hashSetOf<Class<*>>(
+    Boolean::class.java,
+    Char::class.java,
+    Byte::class.java,
+    Short::class.java,
+    Int::class.java,
+    Long::class.java,
+    Float::class.java,
+    Double::class.java,
+    Void::class.java)
+}

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

@@ -40,14 +40,19 @@ data class FactorySchemaAndDescriptor(val schemas: SerializationSchemas, val typ
 // TODO: need to support super classes as well as interfaces with our current code base... what's involved?  If we continue to ban, what is the impact?
 @ThreadSafe
 open class SerializerFactory(
-        val whitelist: ClassWhitelist,
-        cl: ClassLoader,
-        private val evolutionSerializerGetter: EvolutionSerializerGetterBase = EvolutionSerializerGetter()) {
+    val whitelist: ClassWhitelist,
+    cl: ClassLoader,
+    private val evolutionSerializerGetter: EvolutionSerializerGetterBase = EvolutionSerializerGetter()
+) {
     private val serializersByType = ConcurrentHashMap<Type, AMQPSerializer<Any>>()
     private val serializersByDescriptor = ConcurrentHashMap<Any, AMQPSerializer<Any>>()
     private val customSerializers = CopyOnWriteArrayList<SerializerFor>()
     private val transformsCache = ConcurrentHashMap<String, EnumMap<TransformTypes, MutableList<Transform>>>()
 
+    init {
+        fingerPrinter.setOwner(this)
+    }
+
     open val classCarpenter = ClassCarpenter(cl, whitelist)
 
     val classloader: ClassLoader

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/ClassCarpenter.kt

@@ -79,8 +79,10 @@ private val jlClass get() = Type.getInternalName(Class::class.java)
  *
  * Equals/hashCode methods are not yet supported.
  */
-class ClassCarpenter(cl: ClassLoader = Thread.currentThread().contextClassLoader,
-                     val whitelist: ClassWhitelist) {
+class ClassCarpenter(
+    cl: ClassLoader = Thread.currentThread().contextClassLoader,
+    val whitelist: ClassWhitelist
+) {
     // TODO: Generics.
     // TODO: Sandbox the generated code when a security manager is in use.
     // TODO: Generate equals/hashCode.

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

@@ -99,8 +99,10 @@ class StaticInitialisationOfSerializedObjectTest {
 
     // Version of a serializer factory that will allow the class carpenter living on the
     // factory to have a different whitelist applied to it than the factory
-    class TestSerializerFactory(wl1: ClassWhitelist, wl2: ClassWhitelist) :
-            SerializerFactory(wl1, ClassLoader.getSystemClassLoader()) {
+    class TestSerializerFactory(
+            wl1: ClassWhitelist,
+            wl2: ClassWhitelist
+    ) : SerializerFactory(wl1, ClassLoader.getSystemClassLoader()) {
         override val classCarpenter = ClassCarpenter(ClassLoader.getSystemClassLoader(), wl2)
     }
 
@@ -141,4 +143,4 @@ class StaticInitialisationOfSerializedObjectTest {
             DeserializationInput(sf2).deserialize(SerializedBytes<D>(bytes))
         }.isInstanceOf(NotSerializableException::class.java)
     }
-}
+}