Merge pull request #2957 from corda/kat/bug/listSetterSerialization

CORDA-1329 - Setter serialization fails with lists

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 raw type, List for example.
+
 * java.security.cert.CRLReason added to the default Whitelist.
 
 * java.security.cert.X509CRL serialization support added.

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

@@ -305,7 +305,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)))) {
@@ -314,11 +314,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++,

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 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);
+    }
 }

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")
     }
-
 }