From 2fc83b00a384a50060a7ffa634c9019f768d650f Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Thu, 14 Sep 2017 16:27:20 +0100 Subject: [PATCH] CORDA-539 - Integrate enum carpentry into serializer factory (#1508) * CORDA-539 - Integrate enum carpentry into serializer factory * CORDA-539 - Review comments Responding to comments about my comments, mostly fixing spelling and punctuation errors --- .../serialization/amqp/EnumSerializer.kt | 2 +- .../serialization/amqp/SerializerFactory.kt | 12 +- .../carpenter/AMQPSchemaExtensions.kt | 19 ++- .../serialization/carpenter/MetaCarpenter.kt | 25 ++-- .../serialization/carpenter/SchemaFields.kt | 1 - .../DeserializeNeedingCarpentryOfEnumsTest.kt | 109 ++++++++++++++++++ ...erializeNeedingCarpentrySimpleTypesTest.kt | 4 +- ...berCompositeSchemaToClassCarpenterTests.kt | 4 +- ...berCompositeSchemaToClassCarpenterTests.kt | 12 +- 9 files changed, 159 insertions(+), 29 deletions(-) create mode 100644 node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentryOfEnumsTest.kt diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EnumSerializer.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EnumSerializer.kt index a11c547a90..6ac728783c 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EnumSerializer.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/EnumSerializer.kt @@ -11,7 +11,7 @@ import java.lang.reflect.Type */ class EnumSerializer(declaredType: Type, declaredClass: Class<*>, factory: SerializerFactory) : AMQPSerializer { override val type: Type = declaredType - override val typeDescriptor = Symbol.valueOf("$DESCRIPTOR_DOMAIN:${fingerprintForType(type, factory)}") + override val typeDescriptor = Symbol.valueOf("$DESCRIPTOR_DOMAIN:${fingerprintForType(type, factory)}")!! private val typeNotation: TypeNotation init { 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 a32a584d61..754927a017 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 @@ -34,7 +34,7 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl: ClassLoader) { private val serializersByType = ConcurrentHashMap>() private val serializersByDescriptor = ConcurrentHashMap>() private val customSerializers = CopyOnWriteArrayList>() - private val classCarpenter = ClassCarpenter(cl) + val classCarpenter = ClassCarpenter(cl) val classloader: ClassLoader get() = classCarpenter.classloader @@ -190,7 +190,7 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl: ClassLoader) { * if not use the [ClassCarpenter] to generate a class to use in it's place */ private fun processSchema(schema: FactorySchemaAndDescriptor, sentinel: Boolean = false) { - val carpenterSchemas = CarpenterSchemas.newInstance() + val metaSchema = CarpenterMetaSchema.newInstance() for (typeNotation in schema.schema.types) { try { val serialiser = processSchemaEntry(typeNotation) @@ -202,13 +202,13 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl: ClassLoader) { getEvolutionSerializer(typeNotation, serialiser) } } catch (e: ClassNotFoundException) { - if (sentinel || (typeNotation !is CompositeType)) throw e - typeNotation.carpenterSchema(classloader, carpenterSchemas = carpenterSchemas) + if (sentinel) throw e + metaSchema.buildFor(typeNotation, classloader) } } - if (carpenterSchemas.isNotEmpty()) { - val mc = MetaCarpenter(carpenterSchemas, classCarpenter) + if (metaSchema.isNotEmpty()) { + val mc = MetaCarpenter(metaSchema, classCarpenter) mc.build() processSchema(schema, true) } diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt index eb1f21f54a..613e180a9d 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/AMQPSchemaExtensions.kt @@ -1,11 +1,12 @@ package net.corda.nodeapi.internal.serialization.carpenter import net.corda.nodeapi.internal.serialization.amqp.CompositeType +import net.corda.nodeapi.internal.serialization.amqp.RestrictedType import net.corda.nodeapi.internal.serialization.amqp.Field as AMQPField import net.corda.nodeapi.internal.serialization.amqp.Schema as AMQPSchema -fun AMQPSchema.carpenterSchema(classloader: ClassLoader) : CarpenterSchemas { - val rtn = CarpenterSchemas.newInstance() +fun AMQPSchema.carpenterSchema(classloader: ClassLoader) : CarpenterMetaSchema { + val rtn = CarpenterMetaSchema.newInstance() types.filterIsInstance().forEach { it.carpenterSchema(classloader, carpenterSchemas = rtn) @@ -38,7 +39,7 @@ fun AMQPField.typeAsString() = if (type == "*") requires[0] else type * on the class path. For testing purposes schema generation can be forced */ fun CompositeType.carpenterSchema(classloader: ClassLoader, - carpenterSchemas: CarpenterSchemas, + carpenterSchemas: CarpenterMetaSchema, force: Boolean = false) { if (classloader.exists(name)) { validatePropertyTypes(classloader) @@ -83,6 +84,18 @@ fun CompositeType.carpenterSchema(classloader: ClassLoader, } } +// This is potentially problematic as we're assuming the only type of restriction we will be +// carpenting for, an enum, but actually trying to split out RestrictedType into something +// more polymorphic is hard. Additionally, to conform to AMQP we're really serialising +// this as a list so... +fun RestrictedType.carpenterSchema(carpenterSchemas: CarpenterMetaSchema) { + val m: MutableMap = mutableMapOf() + + choices.forEach { m[it.name] = EnumField() } + + carpenterSchemas.carpenterSchemas.add(EnumSchema(name = name, fields = m)) +} + // map a pair of (typename, mandatory) to the corresponding class type // where the mandatory AMQP flag maps to the types nullability val typeStrToType: Map, Class> = mapOf( diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MetaCarpenter.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MetaCarpenter.kt index c678dcadde..26c11208ca 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MetaCarpenter.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MetaCarpenter.kt @@ -1,6 +1,7 @@ package net.corda.nodeapi.internal.serialization.carpenter import net.corda.nodeapi.internal.serialization.amqp.CompositeType +import net.corda.nodeapi.internal.serialization.amqp.RestrictedType import net.corda.nodeapi.internal.serialization.amqp.TypeNotation /** @@ -22,13 +23,13 @@ import net.corda.nodeapi.internal.serialization.amqp.TypeNotation * in turn look up all of those classes in the [dependsOn] list, remove their dependency on the newly created class, * and if that list is reduced to zero know we can now generate a [Schema] for them and carpent them up */ -data class CarpenterSchemas( +data class CarpenterMetaSchema( val carpenterSchemas: MutableList, val dependencies: MutableMap>>, val dependsOn: MutableMap>) { companion object CarpenterSchemaConstructor { - fun newInstance(): CarpenterSchemas { - return CarpenterSchemas(mutableListOf(), mutableMapOf(), mutableMapOf()) + fun newInstance(): CarpenterMetaSchema { + return CarpenterMetaSchema(mutableListOf(), mutableMapOf(), mutableMapOf()) } } @@ -42,18 +43,26 @@ data class CarpenterSchemas( fun isEmpty() = carpenterSchemas.isEmpty() fun isNotEmpty() = carpenterSchemas.isNotEmpty() + + // We could make this an abstract method on TypeNotation but that + // would mean the amqp package being "more" infected with carpenter + // specific bits. + fun buildFor(target: TypeNotation, cl: ClassLoader) = when (target) { + is RestrictedType -> target.carpenterSchema(this) + is CompositeType -> target.carpenterSchema(cl, this, false) + } } /** - * Take a dependency tree of [CarpenterSchemas] and reduce it to zero by carpenting those classes that + * Take a dependency tree of [CarpenterMetaSchema] and reduce it to zero by carpenting those classes that * require it. As classes are carpented check for depdency resolution, if now free generate a [Schema] for - * that class and add it to the list of classes ([CarpenterSchemas.carpenterSchemas]) that require + * that class and add it to the list of classes ([CarpenterMetaSchema.carpenterSchemas]) that require * carpenting * * @property cc a reference to the actual class carpenter we're using to constuct classes * @property objects a list of carpented classes loaded into the carpenters class loader */ -abstract class MetaCarpenterBase(val schemas: CarpenterSchemas, val cc: ClassCarpenter = ClassCarpenter()) { +abstract class MetaCarpenterBase(val schemas: CarpenterMetaSchema, val cc: ClassCarpenter = ClassCarpenter()) { val objects = mutableMapOf>() fun step(newObject: Schema) { @@ -82,7 +91,7 @@ abstract class MetaCarpenterBase(val schemas: CarpenterSchemas, val cc: ClassCar get() = cc.classloader } -class MetaCarpenter(schemas: CarpenterSchemas, +class MetaCarpenter(schemas: CarpenterMetaSchema, cc: ClassCarpenter = ClassCarpenter()) : MetaCarpenterBase(schemas, cc) { override fun build() { while (schemas.carpenterSchemas.isNotEmpty()) { @@ -92,7 +101,7 @@ class MetaCarpenter(schemas: CarpenterSchemas, } } -class TestMetaCarpenter(schemas: CarpenterSchemas, +class TestMetaCarpenter(schemas: CarpenterMetaSchema, cc: ClassCarpenter = ClassCarpenter()) : MetaCarpenterBase(schemas, cc) { override fun build() { if (schemas.carpenterSchemas.isEmpty()) return diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt index 33ab42dcd4..effb5d40c4 100644 --- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt +++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SchemaFields.kt @@ -134,5 +134,4 @@ class EnumField : Field(Enum::class.java) { object FieldFactory { fun newInstance(mandatory: Boolean, name: String, field: Class) = if (mandatory) NonNullableField(name, field) else NullableField(name, field) - } diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentryOfEnumsTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentryOfEnumsTest.kt new file mode 100644 index 0000000000..fd02e19754 --- /dev/null +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentryOfEnumsTest.kt @@ -0,0 +1,109 @@ +package net.corda.nodeapi.internal.serialization.amqp + +import org.junit.Test +import kotlin.test.* +import net.corda.nodeapi.internal.serialization.carpenter.* + +class DeserializeNeedingCarpentryOfEnumsTest : AmqpCarpenterBase() { + companion object { + /** + * If you want to see the schema encoded into the envelope after serialisation change this to true + */ + private const val VERBOSE = false + } + + @Test + fun singleEnum() { + // + // Setup the test + // + val setupFactory = testDefaultFactory() + + val enumConstants = listOf("AAA", "BBB", "CCC", "DDD", "EEE", "FFF", + "GGG", "HHH", "III", "JJJ").associateBy({ it }, { EnumField() }) + + // create the enum + val testEnumType = setupFactory.classCarpenter.build(EnumSchema("test.testEnumType", enumConstants)) + + // create the class that has that enum as an element + val testClassType = setupFactory.classCarpenter.build(ClassSchema("test.testClassType", + mapOf("a" to NonNullableField(testEnumType)))) + + // create an instance of the class we can then serialise + val testInstance = testClassType.constructors[0].newInstance(testEnumType.getMethod( + "valueOf", String::class.java).invoke(null, "BBB")) + + // serialise the object + val serialisedBytes = TestSerializationOutput(VERBOSE, setupFactory).serialize(testInstance) + + // + // Test setup done, now on with the actual test + // + + // need a second factory to ensure a second carpenter is used and thus the class we're attempting + // to de-serialise isn't in the factories class loader + val testFactory = testDefaultFactoryWithWhitelist() + + val deserializedObj = DeserializationInput(testFactory).deserialize(serialisedBytes) + + assertTrue(deserializedObj::class.java.getMethod("getA").invoke(deserializedObj)::class.java.isEnum) + assertEquals("BBB", + (deserializedObj::class.java.getMethod("getA").invoke(deserializedObj) as Enum<*>).name) + } + + @Test + fun compositeIncludingEnums() { + // + // Setup the test + // + val setupFactory = testDefaultFactory() + + val enumConstants = listOf("AAA", "BBB", "CCC", "DDD", "EEE", "FFF", + "GGG", "HHH", "III", "JJJ").associateBy({ it }, { EnumField() }) + + // create the enum + val testEnumType1 = setupFactory.classCarpenter.build(EnumSchema("test.testEnumType1", enumConstants)) + val testEnumType2 = setupFactory.classCarpenter.build(EnumSchema("test.testEnumType2", enumConstants)) + + // create the class that has that enum as an element + val testClassType = setupFactory.classCarpenter.build(ClassSchema("test.testClassType", + mapOf( + "a" to NonNullableField(testEnumType1), + "b" to NonNullableField(testEnumType2), + "c" to NullableField(testEnumType1), + "d" to NullableField(String::class.java)))) + + val vOf1 = testEnumType1.getMethod("valueOf", String::class.java) + val vOf2 = testEnumType2.getMethod("valueOf", String::class.java) + val testStr = "so many things [Ø Þ]" + + // create an instance of the class we can then serialise + val testInstance = testClassType.constructors[0].newInstance( + vOf1.invoke(null, "CCC"), + vOf2.invoke(null, "EEE"), + null, + testStr) + + // serialise the object + val serialisedBytes = TestSerializationOutput(VERBOSE, setupFactory).serialize(testInstance) + + // + // Test setup done, now on with the actual test + // + + // need a second factory to ensure a second carpenter is used and thus the class we're attempting + // to de-serialise isn't in the factories class loader + val testFactory = testDefaultFactoryWithWhitelist() + + val deserializedObj = DeserializationInput(testFactory).deserialize(serialisedBytes) + + assertTrue(deserializedObj::class.java.getMethod("getA").invoke(deserializedObj)::class.java.isEnum) + assertEquals("CCC", + (deserializedObj::class.java.getMethod("getA").invoke(deserializedObj) as Enum<*>).name) + assertTrue(deserializedObj::class.java.getMethod("getB").invoke(deserializedObj)::class.java.isEnum) + assertEquals("EEE", + (deserializedObj::class.java.getMethod("getB").invoke(deserializedObj) as Enum<*>).name) + assertNull(deserializedObj::class.java.getMethod("getC").invoke(deserializedObj)) + assertEquals(testStr, deserializedObj::class.java.getMethod("getD").invoke(deserializedObj)) + } +} diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentrySimpleTypesTest.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentrySimpleTypesTest.kt index 0a05542ad7..f368729af9 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentrySimpleTypesTest.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentrySimpleTypesTest.kt @@ -16,8 +16,8 @@ class DeserializeNeedingCarpentrySimpleTypesTest : AmqpCarpenterBase() { private const val VERBOSE = false } - val sf = testDefaultFactory() - val sf2 = testDefaultFactory() + private val sf = testDefaultFactory() + private val sf2 = testDefaultFactory() @Test fun singleInt() { diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MultiMemberCompositeSchemaToClassCarpenterTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MultiMemberCompositeSchemaToClassCarpenterTests.kt index f9ea362882..6c04536a80 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MultiMemberCompositeSchemaToClassCarpenterTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/MultiMemberCompositeSchemaToClassCarpenterTests.kt @@ -35,7 +35,7 @@ class MultiMemberCompositeSchemaToClassCarpenterTests : AmqpCarpenterBase() { assertEquals("b", amqpSchema.fields[1].name) assertEquals("int", amqpSchema.fields[1].type) - val carpenterSchema = CarpenterSchemas.newInstance() + val carpenterSchema = CarpenterMetaSchema.newInstance() amqpSchema.carpenterSchema( classloader = ClassLoader.getSystemClassLoader(), carpenterSchemas = carpenterSchema, @@ -79,7 +79,7 @@ class MultiMemberCompositeSchemaToClassCarpenterTests : AmqpCarpenterBase() { assertEquals("b", amqpSchema.fields[1].name) assertEquals("string", amqpSchema.fields[1].type) - val carpenterSchema = CarpenterSchemas.newInstance() + val carpenterSchema = CarpenterMetaSchema.newInstance() amqpSchema.carpenterSchema( classloader = ClassLoader.getSystemClassLoader(), carpenterSchemas = carpenterSchema, diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SingleMemberCompositeSchemaToClassCarpenterTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SingleMemberCompositeSchemaToClassCarpenterTests.kt index 532e174903..2858ab4845 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SingleMemberCompositeSchemaToClassCarpenterTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/carpenter/SingleMemberCompositeSchemaToClassCarpenterTests.kt @@ -29,7 +29,7 @@ class SingleMemberCompositeSchemaToClassCarpenterTests : AmqpCarpenterBase() { assertEquals("a", amqpSchema.fields[0].name) assertEquals("int", amqpSchema.fields[0].type) - val carpenterSchema = CarpenterSchemas.newInstance() + val carpenterSchema = CarpenterMetaSchema.newInstance() amqpSchema.carpenterSchema( classloader = ClassLoader.getSystemClassLoader(), carpenterSchemas = carpenterSchema, @@ -60,7 +60,7 @@ class SingleMemberCompositeSchemaToClassCarpenterTests : AmqpCarpenterBase() { assert(obj.envelope.schema.types[0] is CompositeType) val amqpSchema = obj.envelope.schema.types[0] as CompositeType - val carpenterSchema = CarpenterSchemas.newInstance() + val carpenterSchema = CarpenterMetaSchema.newInstance() amqpSchema.carpenterSchema( classloader = ClassLoader.getSystemClassLoader(), carpenterSchemas = carpenterSchema, @@ -95,7 +95,7 @@ class SingleMemberCompositeSchemaToClassCarpenterTests : AmqpCarpenterBase() { assertEquals("a", amqpSchema.fields[0].name) assertEquals("long", amqpSchema.fields[0].type) - val carpenterSchema = CarpenterSchemas.newInstance() + val carpenterSchema = CarpenterMetaSchema.newInstance() amqpSchema.carpenterSchema( classloader = ClassLoader.getSystemClassLoader(), carpenterSchemas = carpenterSchema, @@ -130,7 +130,7 @@ class SingleMemberCompositeSchemaToClassCarpenterTests : AmqpCarpenterBase() { assertEquals("a", amqpSchema.fields[0].name) assertEquals("short", amqpSchema.fields[0].type) - val carpenterSchema = CarpenterSchemas.newInstance() + val carpenterSchema = CarpenterMetaSchema.newInstance() amqpSchema.carpenterSchema( classloader = ClassLoader.getSystemClassLoader(), carpenterSchemas = carpenterSchema, @@ -165,7 +165,7 @@ class SingleMemberCompositeSchemaToClassCarpenterTests : AmqpCarpenterBase() { assertEquals("a", amqpSchema.fields[0].name) assertEquals("double", amqpSchema.fields[0].type) - val carpenterSchema = CarpenterSchemas.newInstance() + val carpenterSchema = CarpenterMetaSchema.newInstance() amqpSchema.carpenterSchema( classloader = ClassLoader.getSystemClassLoader(), carpenterSchemas = carpenterSchema, @@ -200,7 +200,7 @@ class SingleMemberCompositeSchemaToClassCarpenterTests : AmqpCarpenterBase() { assertEquals("a", amqpSchema.fields[0].name) assertEquals("float", amqpSchema.fields[0].type) - val carpenterSchema = CarpenterSchemas.newInstance() + val carpenterSchema = CarpenterMetaSchema.newInstance() amqpSchema.carpenterSchema( classloader = ClassLoader.getSystemClassLoader(), carpenterSchemas = carpenterSchema,