CORDA-1115 - Cannot serialize private nested objects (#2665)

* CORDA-1115 - Cannot serialize private nested objects

Shown up by the simm-valuation-demo the problem was where a private
object field of an object was being serialised within the outer objects
context (see tests added for example)

Fix is to switch from Kotlin reflection back to Java.

Additional fix to the test where it was comparing two lists of state
references in a flow and they weren't equal because they weren't in the
same order... This I assume is just an oversight (in that them being
in a different order but otherwise the same is actually fine) so
converting to set comparison

* Fix forward port issue where fingerprinting has moved

* Review Comments

* Review Comments

* Review Comments

* Gran -> Grab

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

@@ -153,7 +153,7 @@ class SerializerFingerPrinter : FingerPrinter {
                             }.putUnencodedChars(type.name).putUnencodedChars(ENUM_HASH)
                         } else {
                             hasher.fingerprintWithCustomSerializerOrElse(factory!!, type, type) {
-                                if (type.kotlin.objectInstance != null) {
+                                if (type.objectInstance() != null) {
                                     // TODO: name collision is too likely for kotlin objects, we need to introduce some reference
                                     // to the CorDapp but maybe reference to the JAR in the short term.
                                     hasher.putUnencodedChars(type.name)

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

@@ -513,3 +513,48 @@ fun ClassWhitelist.hasAnnotationInHierarchy(type: Class<*>): Boolean {
             || type.interfaces.any { hasAnnotationInHierarchy(it) }
             || (type.superclass != null && hasAnnotationInHierarchy(type.superclass))
 }
+
+/**
+ * By default use Kotlin reflection and grab the objectInstance. However, that doesn't play nicely with nested
+ * private objects. Even setting the accessibility override (setAccessible) still causes an
+ * [IllegalAccessException] when attempting to retrieve the value of the INSTANCE field.
+ *
+ * Whichever reference to the class Kotlin reflection uses, override (set from setAccessible) on that field
+ * isn't set even when it was explicitly set as accessible before calling into the kotlin reflection routines.
+ *
+ * For example
+ *
+ * clazz.getDeclaredField("INSTANCE")?.apply {
+ *     isAccessible = true
+ *     kotlin.objectInstance // This throws as the INSTANCE field isn't accessible
+ * }
+ *
+ * Therefore default back to good old java reflection and simply look for the INSTANCE field as we are never going
+ * to serialize a companion object.
+ *
+ * As such, if objectInstance fails access, revert to Java reflection and try that
+ */
+fun Class<*>.objectInstance() =
+        try {
+            this.kotlin.objectInstance
+        } catch (e: IllegalAccessException) {
+            // Check it really is an object (i.e. it has no constructor)
+            if (constructors.isNotEmpty()) null
+            else {
+                try {
+                    this.getDeclaredField("INSTANCE")?.let { field ->
+                        // and must be marked as both static and final (>0 means they're set)
+                        if (modifiers and Modifier.STATIC == 0 || modifiers and Modifier.FINAL == 0) null
+                        else {
+                            val accessibility = field.isAccessible
+                            field.isAccessible = true
+                            val obj = field.get(null)
+                            field.isAccessible = accessibility
+                            obj
+                        }
+                    }
+                } catch (e: NoSuchFieldException) {
+                    null
+                }
+            }
+        }

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

@@ -267,12 +267,15 @@ open class SerializerFactory(
                     // Don't need to check the whitelist since each element will come back through the whitelisting process.
                     if (clazz.componentType.isPrimitive) PrimArraySerializer.make(type, this)
                     else ArraySerializer.make(type, this)
-                } else if (clazz.kotlin.objectInstance != null) {
-                    whitelist.requireWhitelisted(clazz)
-                    SingletonSerializer(clazz, clazz.kotlin.objectInstance!!, this)
                 } else {
-                    whitelist.requireWhitelisted(type)
-                    ObjectSerializer(type, this)
+                    val singleton = clazz.objectInstance()
+                    if (singleton != null) {
+                       whitelist.requireWhitelisted(clazz)
+                       SingletonSerializer(clazz, singleton, this)
+                   } else {
+                       whitelist.requireWhitelisted(type)
+                       ObjectSerializer(type, this)
+                   }
                 }
             }
         }

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

@@ -47,6 +47,22 @@ import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
 
+object AckWrapper {
+    object Ack
+    fun serialize() {
+        val factory = testDefaultFactoryNoEvolution()
+        SerializationOutput(factory).serialize(Ack)
+    }
+}
+
+object PrivateAckWrapper {
+    private object Ack
+    fun serialize() {
+        val factory = testDefaultFactoryNoEvolution()
+        SerializationOutput(factory).serialize(Ack)
+    }
+}
+
 @RunWith(Parameterized::class)
 class SerializationOutputTests(private val compression: CordaSerializationEncoding?) {
     private companion object {
@@ -1148,4 +1164,18 @@ class SerializationOutputTests(private val compression: CordaSerializationEncodi
             assertEquals(encodingNotPermittedFormat.format(compression), message)
         }
     }
-}
+
+    @Test
+    fun nestedObjects() {
+        // The "test" is that this doesn't throw, anything else is a success
+        AckWrapper.serialize()
+    }
+
+    @Test
+    fun privateNestedObjects() {
+        // The "test" is that this doesn't throw, anything else is a success
+        PrivateAckWrapper.serialize()
+    }
+
+}
+

samples/simm-valuation-demo/src/main/kotlin/net/corda/vega/flows/SimmFlow.kt

@@ -305,7 +305,8 @@ object SimmFlow {
         @Suspendable
         private fun agreePortfolio(portfolio: Portfolio): SignedTransaction {
             logger.info("Handshake finished, awaiting Simm offer")
-            require(offer.dealBeingOffered.portfolio == portfolio.refs)
+
+            require(offer.dealBeingOffered.portfolio.toSet() == portfolio.refs.toSet())
 
             val seller = TwoPartyDealFlow.Instigator(
                     replyToSession,