From d6d7fb52b4d108eb8e9667b630ed8c758c837174 Mon Sep 17 00:00:00 2001 From: Viktor Kolomeyko <31008341+vkolomeyko@users.noreply.github.com> Date: Wed, 23 Aug 2017 17:37:49 +0100 Subject: [PATCH] Make sure members of type `Class` correctly serialized using AMQP (#1314) * Minor changes and expose the problem with class serialization * Custom serializer for Class * More changes to make TransactionEncumbranceTests pass in AMQP mode --- .../CommandsSerializationTests.kt | 18 +++++++++++++ .../serialization/AMQPSerializationScheme.kt | 1 + .../amqp/DeserializationInput.kt | 6 ++--- .../internal/serialization/amqp/Schema.kt | 20 +++++++++----- .../serialization/amqp/SerializationHelper.kt | 26 +++++++++---------- .../serialization/amqp/SerializerFactory.kt | 3 ++- .../amqp/custom/ClassSerializer.kt | 15 +++++++++++ .../corda/testing/contracts/DummyContract.kt | 4 +-- 8 files changed, 68 insertions(+), 25 deletions(-) create mode 100644 core/src/test/kotlin/net/corda/core/serialization/CommandsSerializationTests.kt create mode 100644 node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/custom/ClassSerializer.kt diff --git a/core/src/test/kotlin/net/corda/core/serialization/CommandsSerializationTests.kt b/core/src/test/kotlin/net/corda/core/serialization/CommandsSerializationTests.kt new file mode 100644 index 0000000000..5c049c5638 --- /dev/null +++ b/core/src/test/kotlin/net/corda/core/serialization/CommandsSerializationTests.kt @@ -0,0 +1,18 @@ +package net.corda.core.serialization + +import net.corda.finance.contracts.CommercialPaper +import net.corda.finance.contracts.asset.Cash +import net.corda.testing.TestDependencyInjectionBase +import org.junit.Test +import kotlin.test.assertEquals + +class CommandsSerializationTests : TestDependencyInjectionBase() { + + @Test + fun `test cash move serialization`() { + val command = Cash.Commands.Move(CommercialPaper::class.java) + val copiedCommand = command.serialize().deserialize() + + assertEquals(command, copiedCommand) + } +} \ No newline at end of file diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AMQPSerializationScheme.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AMQPSerializationScheme.kt index 94547cd963..dc489c458b 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AMQPSerializationScheme.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AMQPSerializationScheme.kt @@ -36,6 +36,7 @@ abstract class AbstractAMQPSerializationScheme : SerializationScheme { register(net.corda.nodeapi.internal.serialization.amqp.custom.YearMonthSerializer(this)) register(net.corda.nodeapi.internal.serialization.amqp.custom.MonthDaySerializer(this)) register(net.corda.nodeapi.internal.serialization.amqp.custom.PeriodSerializer(this)) + register(net.corda.nodeapi.internal.serialization.amqp.custom.ClassSerializer(this)) } } } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializationInput.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializationInput.kt index 7e2fe9f749..612d196433 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializationInput.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializationInput.kt @@ -131,10 +131,10 @@ class DeserializationInput(internal val serializerFactory: SerializerFactory) { } /** - * TODO: Currently performs rather basic checks aimed in particular at [java.util.List>] + * TODO: Currently performs rather basic checks aimed in particular at [java.util.List>] and + * [java.lang.Class] * In the future tighter control might be needed */ private fun Type.materiallyEquivalentTo(that: Type): Boolean = - asClass() == that.asClass() && this is ParameterizedType && that is ParameterizedType && - actualTypeArguments.size == that.actualTypeArguments.size + asClass() == that.asClass() && that is ParameterizedType } \ No newline at end of file diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt index 27f6ca1bef..11b4d30568 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/Schema.kt @@ -339,6 +339,16 @@ internal fun fingerprintForDescriptors(vararg typeDescriptors: String): String { return hasher.hash().asBytes().toBase64() } +private fun Hasher.fingerprintWithCustomSerializerOrElse(factory: SerializerFactory, clazz: Class<*>, declaredType: Type, block: () -> Hasher) : Hasher { + // Need to check if a custom serializer is applicable + val customSerializer = factory.findCustomSerializer(clazz, declaredType) + return if (customSerializer != null) { + putUnencodedChars(customSerializer.typeDescriptor) + } else { + block() + } +} + // This method concatentates various elements of the types recursively as unencoded strings into the hasher, effectively // creating a unique string for a type which we then hash in the calling function above. private fun fingerprintForType(type: Type, contextType: Type?, alreadySeen: MutableSet, hasher: Hasher, factory: SerializerFactory): Hasher { @@ -357,9 +367,7 @@ private fun fingerprintForType(type: Type, contextType: Type?, alreadySeen: Muta } else if (isCollectionOrMap(type)) { hasher.putUnencodedChars(type.name) } else { - // Need to check if a custom serializer is applicable - val customSerializer = factory.findCustomSerializer(type, type) - if (customSerializer == null) { + hasher.fingerprintWithCustomSerializerOrElse(factory, type, type) { if (type.kotlin.objectInstance != null) { // TODO: name collision is too likely for kotlin objects, we need to introduce some reference // to the CorDapp but maybe reference to the JAR in the short term. @@ -367,8 +375,6 @@ private fun fingerprintForType(type: Type, contextType: Type?, alreadySeen: Muta } else { fingerprintForObject(type, contextType, alreadySeen, hasher, factory) } - } else { - hasher.putUnencodedChars(customSerializer.typeDescriptor) } } } else if (type is ParameterizedType) { @@ -377,7 +383,9 @@ private fun fingerprintForType(type: Type, contextType: Type?, alreadySeen: Muta val startingHash = if (isCollectionOrMap(clazz)) { hasher.putUnencodedChars(clazz.name) } else { - fingerprintForObject(type, type, alreadySeen, hasher, factory) + hasher.fingerprintWithCustomSerializerOrElse(factory, clazz, type) { + fingerprintForObject(type, type, alreadySeen, hasher, factory) + } } // ... and concatentate the type data for each parameter type. type.actualTypeArguments.fold(startingHash) { orig, paramType -> fingerprintForType(paramType, type, alreadySeen, orig, factory) } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt index c2cf4a37ee..b719c956f4 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt @@ -74,7 +74,8 @@ private fun propertiesForSerializationFromConstructor(kotlinConstructo val rc: MutableList = ArrayList(kotlinConstructor.parameters.size) for (param in kotlinConstructor.parameters) { val name = param.name ?: throw NotSerializableException("Constructor parameter of $clazz has no name.") - val matchingProperty = properties[name] ?: throw NotSerializableException("No property matching constructor parameter named $name of $clazz." + + val matchingProperty = properties[name] ?: + throw NotSerializableException("No property matching constructor parameter named $name of $clazz." + " If using Java, check that you have the -parameters option specified in the Java compiler.") // Check that the method has a getter in java. val getter = matchingProperty.readMethod ?: throw NotSerializableException("Property has no getter method for $name of $clazz." + @@ -163,21 +164,20 @@ private fun resolveTypeVariables(actualType: Type, contextType: Type?): Type { } internal fun Type.asClass(): Class<*>? { - return if (this is Class<*>) { - this - } else if (this is ParameterizedType) { - this.rawType.asClass() - } else if (this is GenericArrayType) { - this.genericComponentType.asClass()?.arrayClass() - } else null + return when { + this is Class<*> -> this + this is ParameterizedType -> this.rawType.asClass() + this is GenericArrayType -> this.genericComponentType.asClass()?.arrayClass() + else -> null + } } internal fun Type.asArray(): Type? { - return if (this is Class<*>) { - this.arrayClass() - } else if (this is ParameterizedType) { - DeserializedGenericArrayType(this) - } else null + return when { + this is Class<*> -> this.arrayClass() + this is ParameterizedType -> DeserializedGenericArrayType(this) + else -> null + } } internal fun Class<*>.arrayClass(): Class<*> = java.lang.reflect.Array.newInstance(this, 0).javaClass diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt index 5792bb6011..ad8d0c066e 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt @@ -156,7 +156,8 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl : ClassLoader) { fun get(typeDescriptor: Any, schema: Schema): AMQPSerializer { return serializersByDescriptor[typeDescriptor] ?: { processSchema(schema) - serializersByDescriptor[typeDescriptor] ?: throw NotSerializableException("Could not find type matching descriptor $typeDescriptor.") + serializersByDescriptor[typeDescriptor] ?: + throw NotSerializableException("Could not find type matching descriptor $typeDescriptor.") }() } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/custom/ClassSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/custom/ClassSerializer.kt new file mode 100644 index 0000000000..7c399d5a68 --- /dev/null +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/custom/ClassSerializer.kt @@ -0,0 +1,15 @@ +package net.corda.nodeapi.internal.serialization.amqp.custom + +import net.corda.nodeapi.internal.serialization.amqp.CustomSerializer +import net.corda.nodeapi.internal.serialization.amqp.SerializerFactory + +/** + * A serializer for [Class] that uses [ClassProxy] proxy object to write out + */ +class ClassSerializer(factory: SerializerFactory) : CustomSerializer.Proxy, ClassSerializer.ClassProxy>(Class::class.java, ClassProxy::class.java, factory) { + override fun toProxy(obj: Class<*>): ClassProxy = ClassProxy(obj.name) + + override fun fromProxy(proxy: ClassProxy): Class<*> = Class.forName(proxy.className, true, factory.classloader) + + data class ClassProxy(val className: String) +} \ No newline at end of file diff --git a/test-utils/src/main/kotlin/net/corda/testing/contracts/DummyContract.kt b/test-utils/src/main/kotlin/net/corda/testing/contracts/DummyContract.kt index 63cad3f3c6..14f80dee98 100644 --- a/test-utils/src/main/kotlin/net/corda/testing/contracts/DummyContract.kt +++ b/test-utils/src/main/kotlin/net/corda/testing/contracts/DummyContract.kt @@ -6,11 +6,11 @@ import net.corda.core.identity.Party import net.corda.core.transactions.LedgerTransaction import net.corda.core.transactions.TransactionBuilder -// The dummy contract doesn't do anything useful. It exists for testing purposes. +// The dummy contract doesn't do anything useful. It exists for testing purposes, but has to be serializable val DUMMY_PROGRAM_ID = DummyContract() -data class DummyContract(private val blank: Void? = null) : Contract { +data class DummyContract(val blank: Any? = null) : Contract { interface State : ContractState { val magicNumber: Int }