Merge pull request #1210 from corda/feature/kat/changeContainerSerialisation

Change container serialisation to allow concrete types in classes

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/AllButBlacklisted.kt

@@ -39,6 +39,9 @@ object AllButBlacklisted : ClassWhitelist {
             Thread::class.java.name,
             HashSet::class.java.name,
             HashMap::class.java.name,
+            WeakHashMap::class.java.name,
+            Dictionary::class.java.name, // Deprecated (marked obsolete) in the jdk
+            Hashtable::class.java.name, // see [Dictionary]
             ClassLoader::class.java.name,
             Handler::class.java.name, // MemoryHandler, StreamHandler
             Runtime::class.java.name,

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/MapSerializer.kt

@@ -5,6 +5,7 @@ import java.io.NotSerializableException
 import java.lang.reflect.ParameterizedType
 import java.lang.reflect.Type
 import java.util.*
+import kotlin.collections.LinkedHashMap
 import kotlin.collections.Map
 import kotlin.collections.iterator
 import kotlin.collections.map
@@ -17,12 +18,15 @@ class MapSerializer(val declaredType: ParameterizedType, factory: SerializerFact
     override val typeDescriptor = "$DESCRIPTOR_DOMAIN:${fingerprintForType(type, factory)}"
 
     companion object {
-        private val supportedTypes: Map<Class<out Map<*, *>>, (Map<*, *>) -> Map<*, *>> = mapOf(
+        private val supportedTypes: Map<Class<out Any?>, (Map<*, *>) -> Map<*, *>> = mapOf(
+                // Interfaces
                 Map::class.java to { map -> Collections.unmodifiableMap(map) },
                 SortedMap::class.java to { map -> Collections.unmodifiableSortedMap(TreeMap(map)) },
-                NavigableMap::class.java to { map -> Collections.unmodifiableNavigableMap(TreeMap(map)) }
+                NavigableMap::class.java to { map -> Collections.unmodifiableNavigableMap(TreeMap(map)) },
+                // concrete classes for user convenience
+                LinkedHashMap::class.java to { map -> LinkedHashMap(map) },
+                TreeMap::class.java to { map -> TreeMap(map) }
         )
-
         private fun findConcreteType(clazz: Class<*>): (Map<*, *>) -> Map<*, *> {
             return supportedTypes[clazz] ?: throw NotSerializableException("Unsupported map type $clazz.")
         }
@@ -40,7 +44,7 @@ class MapSerializer(val declaredType: ParameterizedType, factory: SerializerFact
     }
 
     override fun writeObject(obj: Any, data: Data, type: Type, output: SerializationOutput) {
-        obj.javaClass.checkNotUnorderedHashMap()
+        obj.javaClass.checkNotUnsupportedHashMap()
         // Write described
         data.withDescribed(typeNotation.descriptor) {
             // Write map
@@ -65,8 +69,17 @@ class MapSerializer(val declaredType: ParameterizedType, factory: SerializerFact
                     input.readObjectOrNull(entry.value, schema, declaredType.actualTypeArguments[1])
 }
 
-internal fun Class<*>.checkNotUnorderedHashMap() {
+internal fun Class<*>.checkNotUnsupportedHashMap() {
     if (HashMap::class.java.isAssignableFrom(this) && !LinkedHashMap::class.java.isAssignableFrom(this)) {
-        throw IllegalArgumentException("Map type $this is unstable under iteration. Suggested fix: use java.util.LinkedHashMap instead.")
+        throw IllegalArgumentException(
+                "Map type $this is unstable under iteration. Suggested fix: use java.util.LinkedHashMap instead.")
+    }
+    else if (WeakHashMap::class.java.isAssignableFrom(this)) {
+        throw IllegalArgumentException ("Weak references with map types not supported. Suggested fix: "
+                                        + "use java.util.LinkedHashMap instead.")
+    }
+    else if (Dictionary::class.java.isAssignableFrom(this)) {
+        throw IllegalArgumentException (
+                "Unable to serialise deprecated type $this. Suggested fix: prefer java.util.map implementations")
     }
 }

node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializerFactory.kt

@@ -272,7 +272,7 @@ class SerializerFactory(val whitelist: ClassWhitelist, cl : ClassLoader) {
 
     private fun makeMapSerializer(declaredType: ParameterizedType): AMQPSerializer<Any> {
         val rawType = declaredType.rawType as Class<*>
-        rawType.checkNotUnorderedHashMap()
+        rawType.checkNotUnsupportedHashMap()
         return MapSerializer(declaredType, this)
     }
 

node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/DeserializeMapTests.kt

@@ -0,0 +1,118 @@
+package net.corda.nodeapi.internal.serialization.amqp
+
+import org.junit.Test
+import java.util.*
+
+class DeserializeCollectionTests {
+    companion object {
+        /**
+         * If you want to see the schema encoded into the envelope after serialisation change this to true
+         */
+        private const val VERBOSE = false
+    }
+
+    val sf = testDefaultFactory()
+
+    @Test
+    fun mapTest() {
+        data class C(val c: Map<String, Int>)
+        val c = C (mapOf("A" to 1, "B" to 2))
+
+        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
+        DeserializationInput(sf).deserialize(serialisedBytes)
+    }
+
+    @Test(expected=java.io.NotSerializableException::class)
+    fun abstractMapFromMapOf() {
+        data class C(val c: AbstractMap<String, Int>)
+        val c = C (mapOf("A" to 1, "B" to 2) as AbstractMap)
+
+        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
+        DeserializationInput(sf).deserialize(serialisedBytes)
+    }
+
+    @Test(expected=java.io.NotSerializableException::class)
+    fun abstractMapFromTreeMap() {
+        data class C(val c: AbstractMap<String, Int>)
+        val c = C (TreeMap(mapOf("A" to 1, "B" to 2)))
+
+        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
+        DeserializationInput(sf).deserialize(serialisedBytes)
+    }
+
+    @Test
+    fun sortedMapTest() {
+        data class C(val c: SortedMap<String, Int>)
+        val c = C(sortedMapOf ("A" to 1, "B" to 2))
+        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
+        DeserializationInput(sf).deserialize(serialisedBytes)
+    }
+
+    @Test
+    fun navigableMapTest() {
+        data class C(val c: NavigableMap<String, Int>)
+        val c = C(TreeMap (mapOf("A" to 1, "B" to 2)).descendingMap())
+
+        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
+        DeserializationInput(sf).deserialize(serialisedBytes)
+    }
+
+    @Test(expected=java.io.NotSerializableException::class)
+    fun dictionaryTest() {
+        data class C(val c: Dictionary<String, Int>)
+        val v : Hashtable<String, Int> = Hashtable()
+        v.put ("a", 1)
+        v.put ("b", 2)
+        val c = C(v)
+
+        // expected to throw
+        TestSerializationOutput(VERBOSE, sf).serialize(c)
+    }
+
+    @Test(expected=java.lang.IllegalArgumentException::class)
+    fun hashtableTest() {
+        data class C(val c: Hashtable<String, Int>)
+        val v : Hashtable<String, Int> = Hashtable()
+        v.put ("a", 1)
+        v.put ("b", 2)
+        val c = C(v)
+
+        // expected to throw
+        TestSerializationOutput(VERBOSE, sf).serialize(c)
+    }
+
+    @Test(expected=java.lang.IllegalArgumentException::class)
+    fun hashMapTest() {
+        data class C(val c : HashMap<String, Int>)
+        val c = C (HashMap (mapOf("A" to 1, "B" to 2)))
+
+        // expect this to throw
+        TestSerializationOutput(VERBOSE, sf).serialize(c)
+    }
+
+    @Test(expected=java.lang.IllegalArgumentException::class)
+    fun weakHashMapTest() {
+        data class C(val c : WeakHashMap<String, Int>)
+        val c = C (WeakHashMap (mapOf("A" to 1, "B" to 2)))
+
+        TestSerializationOutput(VERBOSE, sf).serialize(c)
+    }
+
+    @Test
+    fun concreteTreeMapTest() {
+        data class C(val c: TreeMap<String, Int>)
+        val c = C(TreeMap (mapOf("A" to 1, "B" to 3)))
+
+        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
+        DeserializationInput(sf).deserialize(serialisedBytes)
+    }
+
+    @Test
+    fun concreteLinkedHashMapTest() {
+        data class C(val c : LinkedHashMap<String, Int>)
+        val c = C (LinkedHashMap (mapOf("A" to 1, "B" to 2)))
+
+        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
+        DeserializationInput(sf).deserialize(serialisedBytes)
+    }
+}

node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt

@@ -356,7 +356,7 @@ class SerializationOutputTests {
         serdes(obj)
     }
 
-    @Test(expected = NotSerializableException::class)
+    @Test
     fun `test TreeMap property`() {
         val obj = TreeMapWrapper(TreeMap<Int, Foo>())
         obj.tree[456] = Foo("Fred", 123)