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

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<Any> {
     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 {

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<Type, AMQPSerializer<Any>>()
     private val serializersByDescriptor = ConcurrentHashMap<Any, AMQPSerializer<Any>>()
     private val customSerializers = CopyOnWriteArrayList<CustomSerializer<out Any>>()
-    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
+                if (sentinel) throw e
-                typeNotation.carpenterSchema(classloader, carpenterSchemas = carpenterSchemas)
+                metaSchema.buildFor(typeNotation, classloader)
             }
         }
 
-        if (carpenterSchemas.isNotEmpty()) {
+        if (metaSchema.isNotEmpty()) {
-            val mc = MetaCarpenter(carpenterSchemas, classCarpenter)
+            val mc = MetaCarpenter(metaSchema, classCarpenter)
             mc.build()
             processSchema(schema, true)
         }

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 {
+fun AMQPSchema.carpenterSchema(classloader: ClassLoader) : CarpenterMetaSchema {
-    val rtn = CarpenterSchemas.newInstance()
+    val rtn = CarpenterMetaSchema.newInstance()
 
     types.filterIsInstance<CompositeType>().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<String, Field> = 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<Pair<String, Boolean>, Class<out Any?>> = mapOf(

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<Schema>,
         val dependencies: MutableMap<String, Pair<TypeNotation, MutableList<String>>>,
         val dependsOn: MutableMap<String, MutableList<String>>) {
     companion object CarpenterSchemaConstructor {
-        fun newInstance(): CarpenterSchemas {
+        fun newInstance(): CarpenterMetaSchema {
-            return CarpenterSchemas(mutableListOf(), mutableMapOf(), mutableMapOf())
+            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<String, Class<*>>()
 
     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

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<out Any?>) =
             if (mandatory) NonNullableField(name, field) else NullableField(name, field)
-
 }

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))
+    }
+}

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()
+    private val sf  = testDefaultFactory()
-    val sf2 = testDefaultFactory()
+    private val sf2 = testDefaultFactory()
 
     @Test
     fun singleInt() {

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,

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,