From f19bcea82f53d056963f56554910a07a27a318a5 Mon Sep 17 00:00:00 2001
From: Katelyn Baker <katelyn.baker@r3.com>
Date: Wed, 11 Apr 2018 18:32:23 +0100
Subject: [PATCH] CORDA-1229 - Setter serialization fails with lists

Looks like the super / sub type inference of setter param vs
getter param is the wrong way around. Also, Setter Type should
be the generic type, not just the type

the property must be a supertype of the setter parameter

the getter must be a supertype of the setter parameter
---
 docs/source/changelog.rst                     |  3 ++
 .../serialization/amqp/SerializationHelper.kt |  7 ++--
 .../amqp/SetterConstructorTests.java          | 34 +++++++++++++++++++
 .../amqp/SerializationOutputTests.kt          |  1 -
 4 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
index 9e239532b1..6887eab922 100644
--- a/docs/source/changelog.rst
+++ b/docs/source/changelog.rst
@@ -7,6 +7,9 @@ release, see :doc:`upgrade-notes`.
 Unreleased
 ==========
 
+* Fix CORDA-1229. Setter based serialization was broken with generic types when the property was stored
+  as the interface type, List for example. 
+
 * java.security.cert.CRLReason added to the default Whitelist.
 
 * java.security.cert.X509CRL serialization support added.
diff --git a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt
index f9d9efad63..3b8b6dceff 100644
--- a/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt
+++ b/node-api/src/main/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationHelper.kt
@@ -305,7 +305,8 @@ fun propertiesForSerializationFromSetters(
                         "takes too many arguments")
             }
 
-            val setterType = setter.parameterTypes[0]!!
+            //val setterType = setter.parameterTypes[0]!!
+            val setterType = setter.genericParameterTypes[0]!!
 
             if ((property.value.field != null) &&
                     (!(TypeToken.of(property.value.field?.genericType!!).isSupertypeOf(setterType)))) {
@@ -315,10 +316,10 @@ fun propertiesForSerializationFromSetters(
             }
 
             // make sure the setter returns the same type (within inheritance bounds) the getter accepts
-            if (!(TypeToken.of (setterType).isSupertypeOf(getter.returnType))) {
+            if (!(TypeToken.of (getter.genericReturnType).isSupertypeOf(setterType))) {
                 throw NotSerializableException("Defined setter for parameter ${property.value.field?.name} " +
                         "takes parameter of type $setterType yet the defined getter returns a value of type " +
-                        "${getter.returnType}")
+                        "${getter.returnType} [${getter.genericReturnType}]")
             }
             this += PropertyAccessorGetterSetter(
                     idx++,
diff --git a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/SetterConstructorTests.java b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/SetterConstructorTests.java
index a0ab7276dd..2c2b938ab9 100644
--- a/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/SetterConstructorTests.java
+++ b/node-api/src/test/java/net/corda/nodeapi/internal/serialization/amqp/SetterConstructorTests.java
@@ -7,6 +7,8 @@ import org.junit.Test;
 import static org.junit.Assert.*;
 
 import java.io.NotSerializableException;
+import java.util.ArrayList;
+import java.util.List;
 
 public class SetterConstructorTests {
 
@@ -64,6 +66,13 @@ public class SetterConstructorTests {
         public void setC(int c) { this.c = c; }
     }
 
+    static class CIntList {
+        private List<Integer> l;
+
+        public List getL() { return l; }
+        public void setL(List<Integer> l) { this.l = l; }
+    }
+
     static class Inner1 {
         private String a;
 
@@ -315,4 +324,29 @@ public class SetterConstructorTests {
         Assertions.assertThatThrownBy(() -> new SerializationOutput(factory1).serialize(tm)).isInstanceOf(
                 NotSerializableException.class);
     }
+
+    // This not blowing up means it's working
+    @Test
+    public void intList() throws NotSerializableException {
+        CIntList cil = new CIntList();
+
+        List<Integer> l = new ArrayList<>();
+        l.add(1);
+        l.add(2);
+        l.add(3);
+
+        cil.setL(l);
+
+        EvolutionSerializerGetterBase evolutionSerialiserGetter = new EvolutionSerializerGetter();
+        FingerPrinter fingerPrinter = new SerializerFingerPrinter();
+        SerializerFactory factory1 = new SerializerFactory(
+                AllWhitelist.INSTANCE,
+                ClassLoader.getSystemClassLoader(),
+                evolutionSerialiserGetter,
+                fingerPrinter);
+
+        // if we've got super / sub types on the setter vs the unerlaying type the wrong way around this will
+        // explode
+        new SerializationOutput(factory1).serialize(cil);
+    }
 }
diff --git a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt
index a41f5b33c0..5b10cde6a9 100644
--- a/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt
+++ b/node-api/src/test/kotlin/net/corda/nodeapi/internal/serialization/amqp/SerializationOutputTests.kt
@@ -1310,6 +1310,5 @@ class SerializationOutputTests(private val compression: CordaSerializationEncodi
             C(12).serializeE()
         }.withMessageContaining("has synthetic fields and is likely a nested inner class")
     }
-
 }