Fix AMQP object graph alignment bug and an issue with private constru… (#1376)

* Fix AMQP object graph alignment bug and an issue with private constructors.

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

@@ -3,7 +3,10 @@ package net.corda.nodeapi.internal.serialization.amqp
 import net.corda.core.internal.getStackTraceAsString
 import net.corda.core.serialization.SerializedBytes
 import net.corda.core.utilities.ByteSequence
-import org.apache.qpid.proton.amqp.*
+import org.apache.qpid.proton.amqp.Binary
+import org.apache.qpid.proton.amqp.DescribedType
+import org.apache.qpid.proton.amqp.UnsignedByte
+import org.apache.qpid.proton.amqp.UnsignedInteger
 import org.apache.qpid.proton.codec.Data
 import java.io.NotSerializableException
 import java.lang.reflect.ParameterizedType
@@ -120,7 +123,7 @@ class DeserializationInput(internal val serializerFactory: SerializerFactory) {
                         "is outside of the bounds for the list of size: ${objectHistory.size}")
 
             val objectRetrieved = objectHistory[objectIndex]
-            if (!objectRetrieved::class.java.isSubClassOf(type))
+            if (!objectRetrieved::class.java.isSubClassOf(type.asClass()!!))
                 throw NotSerializableException("Existing reference type mismatch. Expected: '$type', found: '${objectRetrieved::class.java}'")
             objectRetrieved
         }
@@ -138,7 +141,8 @@ class DeserializationInput(internal val serializerFactory: SerializerFactory) {
                 else -> obj // this will be the case for primitive types like [boolean] et al.
             }
             // Store the reference in case we need it later on.
-            objectHistory.add(objectRead)
+            // Skip for primitive types as they are too small and overhead of referencing them will be much higher than their content
+            if (type.asClass()?.isPrimitive != true) objectHistory.add(objectRead)
             objectRead
         }
 

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

@@ -14,7 +14,7 @@ import kotlin.reflect.jvm.javaConstructor
 open class ObjectSerializer(val clazz: Type, factory: SerializerFactory) : AMQPSerializer<Any> {
     override val type: Type get() = clazz
     open val kotlinConstructor = constructorForDeserialization(clazz)
-    val javaConstructor by lazy { kotlinConstructor?.javaConstructor }
+    val javaConstructor by lazy { kotlinConstructor?.javaConstructor?.apply { isAccessible = true } }
 
     private val logger = loggerFor<ObjectSerializer>()
 

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

@@ -87,9 +87,7 @@ open class SerializationOutput(internal val serializerFactory: SerializerFactory
             // Important to do it after serialization such that dependent object will have preceding reference numbers
             // assigned to them first as they will be first read from the stream on receiving end.
             // Skip for primitive types as they are too small and overhead of referencing them will be much higher than their content
-            if(type is Class<*> && !type.isPrimitive) {
+            if (type.asClass()?.isPrimitive != true) objectHistory.put(obj, objectHistory.size)
-                objectHistory.put(obj, objectHistory.size)
-            }
         }
         else {
             data.writeReferencedObject(ReferencedObject(retrievedRefCount))

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

@@ -12,17 +12,17 @@ import net.corda.core.serialization.CordaSerializable
 import net.corda.core.transactions.LedgerTransaction
 import net.corda.nodeapi.RPCException
 import net.corda.nodeapi.internal.serialization.AbstractAMQPSerializationScheme
+import net.corda.nodeapi.internal.serialization.AllWhitelist
 import net.corda.nodeapi.internal.serialization.EmptyWhitelist
 import net.corda.nodeapi.internal.serialization.amqp.SerializerFactory.Companion.isPrimitive
-import net.corda.nodeapi.internal.serialization.AllWhitelist
 import net.corda.testing.BOB_IDENTITY
 import net.corda.testing.MEGA_CORP
 import net.corda.testing.MEGA_CORP_PUBKEY
 import org.apache.qpid.proton.amqp.*
 import org.apache.qpid.proton.codec.DecoderImpl
 import org.apache.qpid.proton.codec.EncoderImpl
-import org.junit.Ignore
 import org.junit.Assert.assertSame
+import org.junit.Ignore
 import org.junit.Test
 import java.io.IOException
 import java.io.NotSerializableException
@@ -794,4 +794,44 @@ class SerializationOutputTests {
         val bCopy = serdes(nodeB)
         assertEquals("A", bCopy.children.single().content)
     }
+
+    data class Bob(val byteArrays: List<ByteArray>)
+
+    @Ignore("Causes DeserializedParameterizedType.make() to fail")
+    @Test
+    fun `test list of byte arrays`() {
+        val a = ByteArray(1)
+        val b = ByteArray(2)
+        val obj = Bob(listOf(a, b, a))
+
+        val factory = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
+        val factory2 = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
+        serdes(obj, factory, factory2)
+    }
+
+    data class Vic(val a: List<String>, val b: List<String>)
+
+    @Test
+    fun `test generics ignored from graph logic`() {
+        val a = listOf("a", "b")
+        val obj = Vic(a, a)
+
+        val factory = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
+        val factory2 = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
+        val objCopy = serdes(obj, factory, factory2)
+        assertSame(objCopy.a, objCopy.b)
+    }
+
+    data class Spike private constructor(val a: String) {
+        constructor() : this("a")
+    }
+
+    @Test
+    fun `test private constructor`() {
+        val obj = Spike()
+
+        val factory = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
+        val factory2 = SerializerFactory(AllWhitelist, ClassLoader.getSystemClassLoader())
+        serdes(obj, factory, factory2)
+    }
 }