diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializer.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializer.kt index 79e576c0bb..886172b3e4 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializer.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/EvolutionSerializer.kt @@ -14,6 +14,7 @@ import java.lang.reflect.Type import kotlin.reflect.KFunction import kotlin.reflect.full.findAnnotation import kotlin.reflect.jvm.javaType +import kotlin.reflect.jvm.jvmErasure /** @@ -117,12 +118,29 @@ abstract class EvolutionSerializer( readersAsSerialized: Map): AMQPSerializer { val constructorArgs = arrayOfNulls(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[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) @@ -151,8 +169,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 { + fun make(old: CompositeType, + new: ObjectSerializer, + factory: SerializerFactory + ): AMQPSerializer { // The order in which the properties were serialised is important and must be preserved val readersAsSerialized = LinkedHashMap() old.fields.forEach { diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializationHelper.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializationHelper.kt index 65b07d2638..9e9edffbf7 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializationHelper.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializationHelper.kt @@ -540,3 +540,18 @@ fun hasCordaSerializable(type: Class<*>): Boolean { || type.interfaces.any(::hasCordaSerializable) || (type.superclass != null && hasCordaSerializable(type.superclass)) } + +fun isJavaPrimitive(type: Class<*>) = type in JavaPrimitiveTypes.primativeTypes + +private object JavaPrimitiveTypes { + val primativeTypes = hashSetOf>( + 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) +} diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializerFactory.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializerFactory.kt index b45336bc2c..8871907eed 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializerFactory.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/SerializerFactory.kt @@ -78,7 +78,7 @@ open class SerializerFactory( lenientCarpenter: Boolean = false, evolutionSerializerGetter: EvolutionSerializerGetterBase = EvolutionSerializerGetter(), fingerPrinter: FingerPrinter = SerializerFingerPrinter() - ) : this(whitelist, ClassCarpenterImpl(classLoader, whitelist, lenientCarpenter), evolutionSerializerGetter, fingerPrinter) + ) : this(whitelist, ClassCarpenterImpl(whitelist, classLoader, lenientCarpenter), evolutionSerializerGetter, fingerPrinter) init { fingerPrinter.setOwner(this) diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/carpenter/ClassCarpenter.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/carpenter/ClassCarpenter.kt index 742f34482a..315ad35a50 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/carpenter/ClassCarpenter.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/carpenter/ClassCarpenter.kt @@ -104,10 +104,10 @@ interface ClassCarpenter { * Equals/hashCode methods are not yet supported. */ @DeleteForDJVM -class ClassCarpenterImpl(cl: ClassLoader = Thread.currentThread().contextClassLoader, - override val whitelist: ClassWhitelist, - private val lenient: Boolean = false) : ClassCarpenter { - +class ClassCarpenterImpl @JvmOverloads constructor (override val whitelist: ClassWhitelist, + cl: ClassLoader = Thread.currentThread().contextClassLoader, + private val lenient: Boolean = false +) : ClassCarpenter { // TODO: Generics. // TODO: Sandbox the generated code when a security manager is in use. // TODO: Generate equals/hashCode. diff --git a/serialization/src/test/java/net/corda/serialization/internal/amqp/JavaEvolutionTests.java b/serialization/src/test/java/net/corda/serialization/internal/amqp/JavaEvolutionTests.java new file mode 100644 index 0000000000..fa8449840e --- /dev/null +++ b/serialization/src/test/java/net/corda/serialization/internal/amqp/JavaEvolutionTests.java @@ -0,0 +1,100 @@ +package net.corda.serialization.internal.amqp; + +import kotlin.Suppress; +import net.corda.core.serialization.SerializedBytes; +import net.corda.serialization.internal.amqp.testutils.AMQPTestUtilsKt; +import net.corda.serialization.internal.amqp.testutils.TestSerializationContext; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import java.io.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class JavaEvolutionTests { + @Rule + public final ExpectedException exception = ExpectedException.none(); + + // Class as it was when it was serialized and written to disk. Uncomment + // if the test referencing the object needs regenerating. + /* + static class N1 { + private String word; + public N1(String word) { this.word = word; } + public String getWord() { return word; } + } + */ + // Class as it exists now with the newly added element + static class N1 { + private String word; + private Integer wibble; + + public N1(String word, Integer wibble) { + this.word = word; + this.wibble = wibble; + } + public String getWord() { return word; } + public Integer getWibble() { return wibble; } + } + + // Class as it was when it was serialized and written to disk. Uncomment + // if the test referencing the object needs regenerating. + /* + static class N2 { + private String word; + public N2(String word) { this.word = word; } + public String getWord() { return word; } + } + */ + + // Class as it exists now with the newly added element + @SuppressWarnings("unused") + static class N2 { + private String word; + private float wibble; + + public N2(String word, float wibble) { + this.word = word; + this.wibble = wibble; + } + public String getWord() { return word; } + public float getWibble() { return wibble; } + } + + SerializerFactory factory = AMQPTestUtilsKt.testDefaultFactory(); + + @Test + public void testN1AddsNullableInt() throws IOException { + // Uncomment to regenerate the base state of the test + /* + N1 n = new N1("potato"); + AMQPTestUtilsKt.writeTestResource(this, new SerializationOutput(factory).serialize( + n, TestSerializationContext.testSerializationContext)); + */ + + N1 n2 = new DeserializationInput(factory).deserialize( + new SerializedBytes<>(AMQPTestUtilsKt.readTestResource(this)), + N1.class, + TestSerializationContext.testSerializationContext); + assertEquals(n2.getWord(), "potato"); + assertNull(n2.getWibble()); + } + + @Test + public void testN2AddsPrimitive() throws IOException { + // Uncomment to regenerate the base state of the test + /* + N2 n = new N2("This is only a test"); + + AMQPTestUtilsKt.writeTestResource(this, new SerializationOutput(factory).serialize( + n, TestSerializationContext.testSerializationContext)); + */ + + exception.expect(NotSerializableException.class); + new DeserializationInput(factory).deserialize( + new SerializedBytes<>(AMQPTestUtilsKt.readTestResource(this)), + N2.class, + TestSerializationContext.testSerializationContext); + } +} diff --git a/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/StaticInitialisationOfSerializedObjectTest.kt b/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/StaticInitialisationOfSerializedObjectTest.kt index fa324ea34c..526c52c1c7 100644 --- a/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/StaticInitialisationOfSerializedObjectTest.kt +++ b/serialization/src/test/kotlin/net/corda/serialization/internal/amqp/StaticInitialisationOfSerializedObjectTest.kt @@ -98,7 +98,7 @@ 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, ClassCarpenterImpl(ClassLoader.getSystemClassLoader(), wl2)) + SerializerFactory(wl1, ClassCarpenterImpl(wl2, ClassLoader.getSystemClassLoader())) // This time have the serialization factory and the carpenter use different whitelists @Test diff --git a/serialization/src/test/kotlin/net/corda/serialization/internal/carpenter/CarpenterExceptionTests.kt b/serialization/src/test/kotlin/net/corda/serialization/internal/carpenter/CarpenterExceptionTests.kt index a3f936b4d9..8aeaa99ac7 100644 --- a/serialization/src/test/kotlin/net/corda/serialization/internal/carpenter/CarpenterExceptionTests.kt +++ b/serialization/src/test/kotlin/net/corda/serialization/internal/carpenter/CarpenterExceptionTests.kt @@ -89,7 +89,7 @@ class CarpenterExceptionTests { // carpent that class up. However, when looking at the fields specified as properties of that class // we set the class loader of the ClassCarpenter to reject one of them, resulting in a CarpentryError // which we then want the code to wrap in a NotSerializeableException - val cc = ClassCarpenterImpl(TestClassLoader(listOf(C2::class.jvmName)), AllWhitelist) + val cc = ClassCarpenterImpl(AllWhitelist, TestClassLoader(listOf(C2::class.jvmName))) val factory = TestFactory(cc) Assertions.assertThatThrownBy { diff --git a/serialization/src/test/resources/net/corda/serialization/internal/amqp/JavaEvolutionTests.testN1AddsNullableInt b/serialization/src/test/resources/net/corda/serialization/internal/amqp/JavaEvolutionTests.testN1AddsNullableInt new file mode 100644 index 0000000000..334de45fa2 Binary files /dev/null and b/serialization/src/test/resources/net/corda/serialization/internal/amqp/JavaEvolutionTests.testN1AddsNullableInt differ diff --git a/serialization/src/test/resources/net/corda/serialization/internal/amqp/JavaEvolutionTests.testN2AddsPrimitive b/serialization/src/test/resources/net/corda/serialization/internal/amqp/JavaEvolutionTests.testN2AddsPrimitive new file mode 100644 index 0000000000..ffa5f54f8b Binary files /dev/null and b/serialization/src/test/resources/net/corda/serialization/internal/amqp/JavaEvolutionTests.testN2AddsPrimitive differ