diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertyDescriptor.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertyDescriptor.kt index e68882fd77..f3abc48ee8 100644 --- a/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertyDescriptor.kt +++ b/serialization/src/main/kotlin/net/corda/serialization/internal/amqp/PropertyDescriptor.kt @@ -4,6 +4,7 @@ import com.google.common.reflect.TypeToken import net.corda.core.KeepForDJVM import net.corda.core.internal.isPublic import net.corda.core.serialization.SerializableCalculatedProperty +import net.corda.core.utilities.contextLogger import net.corda.serialization.internal.amqp.MethodClassifier.* import java.lang.reflect.Field import java.lang.reflect.Method @@ -32,21 +33,20 @@ data class PropertyDescriptor(val field: Field?, val setter: Method?, val getter */ fun validate() { getter?.apply { - val getterType = genericReturnType field?.apply { - if (!getterType.isSupertypeOf(genericReturnType)) - throw AMQPNotSerializableException( - declaringClass, - "Defined getter for parameter $name returns type $getterType " + + if (!getter.returnType.boxesOrIsAssignableFrom(field.type)) + throw AMQPNotSerializableException( + declaringClass, + "Defined getter for parameter $name returns type ${getter.returnType} " + "yet underlying type is $genericType") } } setter?.apply { - val setterType = genericParameterTypes[0]!! + val setterType = setter.parameterTypes[0]!! field?.apply { - if (!genericType.isSupertypeOf(setterType)) + if (!field.type.boxesOrIsAssignableFrom(setterType)) throw AMQPNotSerializableException( declaringClass, "Defined setter for parameter $name takes parameter of type $setterType " + @@ -54,7 +54,7 @@ data class PropertyDescriptor(val field: Field?, val setter: Method?, val getter } getter?.apply { - if (!genericReturnType.isSupertypeOf(setterType)) + if (!getter.returnType.boxesOrIsAssignableFrom(setterType)) throw AMQPNotSerializableException( declaringClass, "Defined setter for parameter $name takes parameter of type $setterType, " + @@ -64,6 +64,9 @@ data class PropertyDescriptor(val field: Field?, val setter: Method?, val getter } } +private fun Class<*>.boxesOrIsAssignableFrom(other: Class<*>) = + isAssignableFrom(other) || kotlin.javaPrimitiveType == other + private fun Type.isSupertypeOf(that: Type) = TypeToken.of(this).isSupertypeOf(that) // match an uppercase letter that also has a corresponding lower case equivalent @@ -85,7 +88,7 @@ private val propertyMethodRegex = Regex("(?get|set|is)(?\\p{Lu}.*)") * take a single parameter of a type compatible with exampleProperty and isExampleProperty must * return a boolean */ -internal fun Class.propertyDescriptors(): Map { +internal fun Class.propertyDescriptors(warnInvalid: Boolean = true): Map { val fieldProperties = superclassChain().declaredFields().byFieldName() return superclassChain().declaredMethods() @@ -93,8 +96,9 @@ internal fun Class.propertyDescriptors(): Map): Map = - rawType.propertyDescriptors().asSequence().mapNotNull { (name, descriptor) -> + rawType.propertyDescriptors(warnIfNonComposable).asSequence().mapNotNull { (name, descriptor) -> if (descriptor.field == null || descriptor.getter == null) null else { val paramType = (descriptor.getter.genericReturnType).resolveAgainstContext() @@ -310,7 +310,7 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup, parameter.name to index }.toMap() - return rawType.propertyDescriptors().asSequence().mapNotNull { (name, descriptor) -> + return rawType.propertyDescriptors(warnIfNonComposable).asSequence().mapNotNull { (name, descriptor) -> val normalisedName = when { name in constructorParameterIndices -> name name.decapitalize() in constructorParameterIndices -> name.decapitalize() @@ -353,7 +353,7 @@ internal data class LocalTypeInformationBuilder(val lookup: LocalTypeLookup, } private fun getterSetterProperties(rawType: Class<*>): Sequence> = - rawType.propertyDescriptors().asSequence().mapNotNull { (name, descriptor) -> + rawType.propertyDescriptors(warnIfNonComposable).asSequence().mapNotNull { (name, descriptor) -> if (descriptor.getter == null || descriptor.setter == null || descriptor.field == null) null else { val paramType = descriptor.getter.genericReturnType diff --git a/serialization/src/test/kotlin/net/corda/serialization/internal/SetsSerializationTest.kt b/serialization/src/test/kotlin/net/corda/serialization/internal/SetsSerializationTest.kt index 74f92c2e41..de1378fac4 100644 --- a/serialization/src/test/kotlin/net/corda/serialization/internal/SetsSerializationTest.kt +++ b/serialization/src/test/kotlin/net/corda/serialization/internal/SetsSerializationTest.kt @@ -6,14 +6,15 @@ import net.corda.core.serialization.deserialize import net.corda.core.serialization.serialize import net.corda.node.serialization.kryo.kryoMagic import net.corda.node.services.statemachine.DataSessionMessage +import net.corda.serialization.internal.amqp.propertyDescriptors import net.corda.testing.core.SerializationEnvironmentRule import net.corda.testing.internal.kryoSpecific -import org.junit.Assert.assertArrayEquals -import org.junit.Assert.assertEquals +import org.junit.Assert.* import org.junit.Rule import org.junit.Test import java.io.ByteArrayOutputStream import java.util.* +import org.assertj.core.api.Assertions.assertThat class SetsSerializationTest { private companion object { @@ -64,4 +65,27 @@ class SetsSerializationTest { } assertArrayEquals(output.toByteArray(), serializedForm.bytes) } + + open class P + class VarOfP(var p: Set

) + + /* + See CORDA-2860. + + When a class has a var parameter of type Set, Kotlin generates getters and setters with the following (Java) signatures: + + public Set getP(); + public void setP(Set p); + + The PropertyDescriptor.validate method used to check that the return type of the getter was a supertype of the parameter type of the + setter. Unfortunately, Set is not a strict supertype of Set, so this check would fail, throwing an exception. + + We now check only for compatibility of the erased classes, so the call to propertyDescriptors() below should now succeed, returning the + property descriptor for "p". + */ + @Test + fun `type variance on setter getter pair does not fail validation`() { + assertThat(VarOfP::class.java.propertyDescriptors()).containsKey("p") + } + } diff --git a/serialization/src/test/kotlin/net/corda/serialization/internal/model/LocalTypeModelTests.kt b/serialization/src/test/kotlin/net/corda/serialization/internal/model/LocalTypeModelTests.kt index 7a10fdb560..aa15770638 100644 --- a/serialization/src/test/kotlin/net/corda/serialization/internal/model/LocalTypeModelTests.kt +++ b/serialization/src/test/kotlin/net/corda/serialization/internal/model/LocalTypeModelTests.kt @@ -40,7 +40,7 @@ class LocalTypeModelTests { @Test fun `Primitives and collections`() { - assertInformation>("CollectionHolder") + assertInformation>("CollectionHolder") assertInformation>(""" StringKeyedCollectionHolder(list: List, map: Map, array: List[]): CollectionHolder @@ -85,9 +85,9 @@ class LocalTypeModelTests { fun `interfaces and superclasses`() { assertInformation>("SuperSuper") assertInformation>("Super: SuperSuper") - assertInformation>(""" - Abstract: Super, SuperSuper - a: LocalDateTime[] + assertInformation>(""" + Abstract: Super, SuperSuper + a: String[] b: Double """) assertInformation("""