From 1f9c04544d57fd68a43c118f362575234944d2ee Mon Sep 17 00:00:00 2001 From: Katelyn Baker Date: Fri, 4 Aug 2017 12:44:14 +0100 Subject: [PATCH] Make the carpenter a member of the serialiser factory If it's not then the caroenter's class loader isn't persistent and thus we will be constantly recarpenting objects --- .../serialization/amqp/SerializerFactory.kt | 16 +- .../serialization/carpenter/MetaCarpenter.kt | 8 +- .../amqp/DeserializeNeedingCarpentryTests.kt | 179 ++++++++---------- 3 files changed, 94 insertions(+), 109 deletions(-) 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 f19e9c88b2..40b557f8f4 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 @@ -48,6 +48,7 @@ class SerializerFactory(val whitelist: ClassWhitelist = AllWhitelist) { private val serializersByType = ConcurrentHashMap>() private val serializersByDescriptor = ConcurrentHashMap>() private val customSerializers = CopyOnWriteArrayList>() + private val classCarpenter = ClassCarpenter() /** * Look up, and manufacture if necessary, a serializer for the given type. @@ -171,24 +172,23 @@ class SerializerFactory(val whitelist: ClassWhitelist = AllWhitelist) { } } - private fun processSchema(schema: Schema, cl: ClassLoader = DeserializedParameterizedType::class.java.classLoader) { + private fun processSchema(schema: Schema, sentinal: Boolean = false) { val carpenterSchemas = CarpenterSchemas.newInstance() for (typeNotation in schema.types) { try { - processSchemaEntry(typeNotation, cl) + processSchemaEntry(typeNotation, classCarpenter.classloader) } catch (e: ClassNotFoundException) { - if ((cl != DeserializedParameterizedType::class.java.classLoader) - || (typeNotation !is CompositeType)) throw e - typeNotation.carpenterSchema(carpenterSchemas = carpenterSchemas) + if (sentinal || (typeNotation !is CompositeType)) throw e + typeNotation.carpenterSchema( + classLoaders = listOf (classCarpenter.classloader), carpenterSchemas = carpenterSchemas) } } if (carpenterSchemas.isNotEmpty()) { - val mc = MetaCarpenter(carpenterSchemas) + val mc = MetaCarpenter(carpenterSchemas, classCarpenter) mc.build() - - processSchema(schema, mc.classloader) + processSchema(schema, true) } } 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 460284e078..a334fc9e40 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 @@ -56,8 +56,7 @@ data class CarpenterSchemas ( * @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) { - private val cc = ClassCarpenter() +abstract class MetaCarpenterBase (val schemas : CarpenterSchemas, val cc : ClassCarpenter = ClassCarpenter()) { val objects = mutableMapOf>() fun step(newObject: Schema) { @@ -88,7 +87,8 @@ abstract class MetaCarpenterBase (val schemas : CarpenterSchemas) { get() = cc.classloader } -class MetaCarpenter(schemas: CarpenterSchemas) : MetaCarpenterBase(schemas) { +class MetaCarpenter(schemas : CarpenterSchemas, + cc : ClassCarpenter = ClassCarpenter()) : MetaCarpenterBase(schemas, cc) { override fun build() { while (schemas.carpenterSchemas.isNotEmpty()) { val newObject = schemas.carpenterSchemas.removeAt(0) @@ -98,6 +98,8 @@ class MetaCarpenter(schemas: CarpenterSchemas) : MetaCarpenterBase(schemas) { } class TestMetaCarpenter(schemas: CarpenterSchemas) : MetaCarpenterBase(schemas) { +class TestMetaCarpenter(schemas : CarpenterSchemas, + cc : ClassCarpenter = ClassCarpenter()) : MetaCarpenterBase(schemas, cc) { override fun build() { if (schemas.carpenterSchemas.isEmpty()) return step (schemas.carpenterSchemas.removeAt(0)) diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentryTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentryTests.kt index 096d2fa7a0..18186bbc6f 100644 --- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentryTests.kt +++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeNeedingCarpentryTests.kt @@ -9,24 +9,6 @@ interface I { fun getName() : String } -interface II { - fun returnName() : String -} - -interface III { - fun returnAge() : Int - fun returnThingWithName(): II -} - -interface B { - fun getName() : String -} - -interface BB { - fun getAge() : Int - fun getThingWithName(): II -} - /** * These tests work by having the class carpenter build the classes we serialise and then deserialise. Because * those classes don't exist within the system's Class Loader the deserialiser will be forced to carpent @@ -55,6 +37,36 @@ class DeserializeNeedingCarpentryTests { assertEquals (testVal, deserializedObj::class.java.getMethod("getA").invoke(deserializedObj)) } + @Test + fun repeatedTypesAreRecognised() { + val testValA = 10 + val testValB = 20 + val testValC = 20 + val clazz = ClassCarpenter().build(ClassSchema("oneType", mapOf("a" to NonNullableField(Int::class.java)))) + val concreteA = clazz.constructors[0].newInstance(testValA) + val concreteB = clazz.constructors[0].newInstance(testValB) + val concreteC = clazz.constructors[0].newInstance(testValC) + + val deserialisedA = DeserializationInput(sf).deserialize(TestSerializationOutput(VERBOSE, sf).serialize(concreteA)) + + assertEquals (testValA, deserialisedA::class.java.getMethod("getA").invoke(deserialisedA)) + + val deserialisedB = DeserializationInput(sf).deserialize(TestSerializationOutput(VERBOSE, sf).serialize(concreteB)) + + assertEquals (testValB, deserialisedA::class.java.getMethod("getA").invoke(deserialisedB)) + assertEquals (deserialisedA::class.java, deserialisedB::class.java) + + // C is deseriliased with a different factory, meaning a different class carpenter, so the type + // won't already exist and it will be carpented a second time showing that when A and B are the + // same underlying class that we didn't create a second instance of the class with the + // second deserialisation + val lsf = SerializerFactory() + val deserialisedC = DeserializationInput(lsf).deserialize(TestSerializationOutput(VERBOSE, lsf).serialize(concreteC)) + assertEquals (testValC, deserialisedC::class.java.getMethod("getA").invoke(deserialisedC)) + assertNotEquals (deserialisedA::class.java, deserialisedC::class.java) + assertNotEquals (deserialisedB::class.java, deserialisedC::class.java) + } + @Test fun simpleTypeKnownInterface() { val clazz = ClassCarpenter().build (ClassSchema( @@ -70,6 +82,57 @@ class DeserializeNeedingCarpentryTests { assertEquals(testVal, (deserializedObj as I).getName()) } + @Test + fun arrayOfTypes() { + val clazz = ClassCarpenter().build(ClassSchema("oneType", mapOf("a" to NonNullableField(Int::class.java)))) + + data class Outer (val a : Array) + + val outer = Outer (arrayOf ( + clazz.constructors[0].newInstance(1), + clazz.constructors[0].newInstance(2), + clazz.constructors[0].newInstance(3))) + + val deserializedObj = DeserializationInput(sf).deserialize(TestSerializationOutput(VERBOSE, sf).serialize(outer)) + + assertNotEquals((deserializedObj.a[0])::class.java, (outer.a[0])::class.java) + assertNotEquals((deserializedObj.a[1])::class.java, (outer.a[1])::class.java) + assertNotEquals((deserializedObj.a[2])::class.java, (outer.a[2])::class.java) + + assertEquals((deserializedObj.a[0])::class.java, (deserializedObj.a[1])::class.java) + assertEquals((deserializedObj.a[0])::class.java, (deserializedObj.a[2])::class.java) + assertEquals((deserializedObj.a[1])::class.java, (deserializedObj.a[2])::class.java) + + assertEquals( + outer.a[0]::class.java.getMethod("getA").invoke(outer.a[0]), + deserializedObj.a[0]::class.java.getMethod("getA").invoke(deserializedObj.a[0])) + assertEquals( + outer.a[1]::class.java.getMethod("getA").invoke(outer.a[1]), + deserializedObj.a[1]::class.java.getMethod("getA").invoke(deserializedObj.a[1])) + assertEquals( + outer.a[2]::class.java.getMethod("getA").invoke(outer.a[2]), + deserializedObj.a[2]::class.java.getMethod("getA").invoke(deserializedObj.a[2])) + } + + @Test + fun reusedClasses() { + val cc = ClassCarpenter() + + val innerType = cc.build(ClassSchema("inner", mapOf("a" to NonNullableField(Int::class.java)))) + val outerType = cc.build(ClassSchema("outer", mapOf("a" to NonNullableField(innerType)))) + val inner = innerType.constructors[0].newInstance(1) + val outer = outerType.constructors[0].newInstance(innerType.constructors[0].newInstance(2)) + + val serializedI = TestSerializationOutput(VERBOSE, sf).serialize(inner) + val deserialisedI = DeserializationInput(sf).deserialize(serializedI) + val serialisedO = TestSerializationOutput(VERBOSE, sf).serialize(outer) + val deserialisedO = DeserializationInput(sf).deserialize(serialisedO) + + // ensure out carpented version of inner is reused + assertEquals (deserialisedI::class.java, + (deserialisedO::class.java.getMethod("getA").invoke(deserialisedO))::class.java) + } + @Test fun nestedTypes() { val cc = ClassCarpenter() @@ -128,86 +191,6 @@ class DeserializeNeedingCarpentryTests { } } - // technically this test doesn't test anything (relevent to carpanter / serialiser interaction) since - // all the classes are knwon, what does do is replicate the test below to demonstrate it should - // all work - @Test - fun mapOfKnown() { - class lII (val name: String) : II { - override fun returnName() = name - } - - class lIII (val age: Int, val thingWithName: II): III { - override fun returnAge(): Int = age - override fun returnThingWithName() = thingWithName - } - - data class Wrapper(val IIIs: MutableMap) - val wrapper = Wrapper (mutableMapOf()) - val testData = arrayOf(Pair ("Fred", 12), Pair ("Bob", 50), Pair ("Thirsty", 101)) - - testData.forEach { - wrapper.IIIs[it.first] = lIII(it.second, lII(it.first)) - } - - // Now do the actual test by serialising and deserialising [wrapper] - val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(wrapper) - val deserializedObj = DeserializationInput(sf).deserialize(serialisedBytes) - } - - // TODO This class shows that the problem isn't with the carpented class but a general - // TODO Bug / feature of the code... - /* - @Test - fun linkedHashMapTest() { - data class C(val c : LinkedHashMap) - val c = C (LinkedHashMap (mapOf("A" to 1, "B" to 2))) - - val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c) - val deserializedObj = DeserializationInput(sf).deserialize(serialisedBytes) - - } - */ - - // TODO the problem here is that the wrapper class as created by the serialiser - // TODO contains a [LinkedHashMap] and not a [Map] and we thus can't serialise - // TODO it - Talk to Rick about weather we should be able to or not - /* - @Test - fun mapOfInterfaces() { - val cc = ClassCarpenter() - - val implementsI = cc.build(ClassSchema( - "implementsI", mapOf("name" to NonNullableField(String::class.java)), - interfaces = listOf (B::class.java))) - - val implementsII = cc.build(ClassSchema("ImplementsII", mapOf ( - "age" to NonNullableField(Int::class.java), - "thingWithName" to NullableField(B::class.java)), - interfaces = listOf (BB::class.java))) - - // inline fun getval(reified T : Any) : return T::class.java - - val wrapper = cc.build(ClassSchema("wrapper", mapOf ( - "BBs" to NonNullableField(mutableMapOf()::class.java - )))) - - val tmp : MutableMap = mutableMapOf() - val toSerialise = wrapper.constructors.first().newInstance(tmp) - val testData = arrayOf(Pair ("Fred", 12), Pair ("Bob", 50), Pair ("Thirsty", 101)) - - testData.forEach { - (wrapper.getMethod("getBBs").invoke(toSerialise) as MutableMap)[it.first] = - implementsII.constructors.first().newInstance(it.second, - implementsI.constructors.first().newInstance(it.first) as B) as BB - } - - // Now do the actual test by serialising and deserialising [wrapper] - val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(toSerialise) - val deserializedObj = DeserializationInput(sf).deserialize(serialisedBytes) - } - */ - @Test fun unknownInterface() { val cc = ClassCarpenter()