Rework To better deal with types we won't serialise

* Add depricated jvm types to the blacklist
* Add concrete types for user convenience
* Add better "We won't serialise this because" error handling

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

@@ -1,6 +1,7 @@
 package net.corda.nodeapi.internal.serialization
 
 import net.corda.core.serialization.ClassWhitelist
+import sun.jvm.hotspot.memory.Dictionary
 import sun.misc.Unsafe
 import sun.security.util.Password
 import java.io.*
@@ -39,6 +40,8 @@ object AllButBlacklisted : ClassWhitelist {
             Thread::class.java.name,
             HashSet::class.java.name,
             HashMap::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

@@ -20,20 +20,12 @@ class MapSerializer(val declaredType: ParameterizedType, factory: SerializerFact
     companion object {
         private val supportedTypes: Map<Class<out Any?>, (Map<*, *>) -> Map<*, *>> = mapOf(
                 // Interfaces
-                Map::class.java to { map -> map },
-                SortedMap::class.java to { map -> TreeMap(map) },
-                NavigableMap::class.java to { map -> TreeMap(map) },
-                // Sub types
-                Dictionary::class.java to { map -> Hashtable(map) },
-                AbstractMap::class.java to { map -> LinkedHashMap(map) },
-                // concrete types
+                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)) },
+                // concrete classes for user convenience
                 LinkedHashMap::class.java to { map -> LinkedHashMap(map) },
-                TreeMap::class.java to { map -> TreeMap(map) },
-                Hashtable::class.java to { map -> Hashtable(map) },
-                WeakHashMap::class.java to { map -> WeakHashMap(map) }
-                // Explicitly disallowed
-                //  - HashMap::class.java
-                //  - EnumMap::class.java
+                TreeMap::class.java to { map -> TreeMap(map) }
         )
         private fun findConcreteType(clazz: Class<*>): (Map<*, *>) -> Map<*, *> {
             return supportedTypes[clazz] ?: throw NotSerializableException("Unsupported map type $clazz.")
@@ -52,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
@@ -77,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

@@ -33,7 +33,7 @@ class DeserializeCollectionTests {
         DeserializationInput(sf).deserialize(serialisedBytes)
     }
 
-    @Test
+    @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)
@@ -42,7 +42,7 @@ class DeserializeCollectionTests {
         DeserializationInput(sf).deserialize(serialisedBytes)
     }
 
-    @Test
+    @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)))
@@ -50,6 +50,7 @@ class DeserializeCollectionTests {
         val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
         DeserializationInput(sf).deserialize(serialisedBytes)
     }
+
     @Test
     fun sortedMapTest() {
         data class C(val c: SortedMap<String, Int>)
@@ -67,15 +68,28 @@ class DeserializeCollectionTests {
         DeserializationInput(sf).deserialize(serialisedBytes)
     }
 
-    @Test
+    @Test(expected=java.lang.IllegalArgumentException::class)
     fun dictionaryTest() {
         data class C(val c: Dictionary<String, Int>)
         var v : Hashtable<String, Int> = Hashtable()
         v.put ("a", 1)
         v.put ("b", 2)
         val c = C(v)
-        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
-        DeserializationInput(sf).deserialize(serialisedBytes)
+
+        // expected to throw
+        TestSerializationOutput(VERBOSE, sf).serialize(c)
+    }
+
+    @Test(expected=java.lang.IllegalArgumentException::class)
+    fun hashtableTest() {
+        data class C(val c: Hashtable<String, Int>)
+        var 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)
@@ -87,13 +101,12 @@ class DeserializeCollectionTests {
         TestSerializationOutput(VERBOSE, sf).serialize(c)
     }
 
-    @Test
+    @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)))
 
-        val serialisedBytes = TestSerializationOutput(VERBOSE, sf).serialize(c)
-        DeserializationInput(sf).deserialize(serialisedBytes)
+        TestSerializationOutput(VERBOSE, sf).serialize(c)
     }
 
     @Test