From 7701dad80b21b4e8d9a7f6417c0de2a71b24e1d8 Mon Sep 17 00:00:00 2001
From: Katelyn Baker <katelyn.baker@r3.com>
Date: Tue, 1 May 2018 20:52:39 +0100
Subject: [PATCH] CORDA-1229 - Fix issue with setter-based serialisation
 (#3051)

---
 docs/source/changelog.rst                     | 10 ++++--
 .../serialization/amqp/SerializationHelper.kt |  8 ++---
 .../amqp/SetterConstructorTests.java          | 32 +++++++++++++++++++
 .../amqp/SerializationOutputTests.kt          |  2 +-
 4 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
index b42258f189..74f188ea0b 100644
--- a/docs/source/changelog.rst
+++ b/docs/source/changelog.rst
@@ -1,8 +1,14 @@
 Changelog
 =========
 
-Here are brief summaries of what's changed between each snapshot release. This includes guidance on how to upgrade code
-from the previous milestone release.
+Here's a summary of what's changed in each Corda release. For guidance on how to upgrade code from the previous
+release, see :doc:`upgrade-notes`.
+
+Unreleased
+==========
+
+* Fix CORDA-1229. Setter-based serialization was broken with generic types when the property was stored
+  as the raw type, List for example.
 
 .. _changelog_v3.1:
 
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 936da5d044..7a207f3cd1 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
@@ -296,7 +296,7 @@ fun propertiesForSerializationFromSetters(
                         "takes too many arguments")
             }
 
-            val setterType = setter.parameterTypes[0]!!
+            val setterType = setter.genericParameterTypes[0]!!
 
             if ((property.value.field != null) &&
                     (!(TypeToken.of(property.value.field?.genericType!!).isSupertypeOf(setterType)))) {
@@ -305,11 +305,11 @@ fun propertiesForSerializationFromSetters(
                         "${property.value.field?.genericType!!}")
             }
 
-            // make sure the setter returns the same type (within inheritance bounds) the getter accepts
-            if (!(TypeToken.of (setterType).isSupertypeOf(getter.returnType))) {
+            // Make sure the getter returns the same type (within inheritance bounds) the setter accepts.
+            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 defd86d948..96cf9b3be8 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;
 
@@ -305,4 +314,27 @@ 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 evolutionSerializerGetter = new EvolutionSerializerGetter();
+        SerializerFactory factory1 = new SerializerFactory(
+                AllWhitelist.INSTANCE,
+                ClassLoader.getSystemClassLoader(),
+                evolutionSerializerGetter);
+
+        // if we've got super / sub types on the setter vs the underlying type the wrong way around this will
+        // explode. See CORDA-1229 (https://r3-cev.atlassian.net/browse/CORDA-1229)
+        new DeserializationInput(factory1).deserialize(new SerializationOutput(factory1).serialize(cil), CIntList.class);
+    }
 }
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 641e9503ae..6d106e698a 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
@@ -1146,4 +1146,4 @@ class SerializationOutputTests {
         // The "test" is that this doesn't throw, anything else is a success
         PrivateAckWrapper.serialize()
     }
-}
\ No newline at end of file
+}