CORDA-1907: Allow Corda's shell to deserialise using generic type information. (#3810)

.ci/api-current.txt

@@ -5767,7 +5767,7 @@ public class net.corda.client.jackson.StringToMethodCallParser extends java.lang
   @NotNull
   public final net.corda.client.jackson.StringToMethodCallParser<T>$ParsedMethodCall parse(T, String)
   @NotNull
-  public final Object[] parseArguments(String, java.util.List<? extends kotlin.Pair<String, ? extends Class<?>>>, String)
+  public final Object[] parseArguments(String, java.util.List<? extends kotlin.Pair<String, ? extends reflect.Type>>, String)
   public static final net.corda.client.jackson.StringToMethodCallParser$Companion Companion
 ##
 public static final class net.corda.client.jackson.StringToMethodCallParser$Companion extends java.lang.Object

client/jackson/src/main/kotlin/net/corda/client/jackson/StringToMethodCallParser.kt

@@ -10,6 +10,7 @@ import net.corda.core.CordaException
 import net.corda.core.utilities.contextLogger
 import java.lang.reflect.Constructor
 import java.lang.reflect.Method
+import java.lang.reflect.Type
 import java.util.concurrent.Callable
 import javax.annotation.concurrent.ThreadSafe
 import kotlin.reflect.KClass
@@ -173,7 +174,7 @@ open class StringToMethodCallParser<in T : Any> @JvmOverloads constructor(
         // and fail for that too.
         for ((index, method) in methods.withIndex()) {
             try {
-                val args = parseArguments(name, paramNamesFromMethod(method).zip(method.parameterTypes), argStr)
+                val args = parseArguments(name, paramNamesFromMethod(method).zip(method.genericParameterTypes), argStr)
                 return ParsedMethodCall(target, method, args)
             } catch (e: UnparseableCallException) {
                 if (index == methods.size - 1)
@@ -189,15 +190,16 @@ open class StringToMethodCallParser<in T : Any> @JvmOverloads constructor(
      * @param methodNameHint A name that will be used in exceptions if thrown; not used for any other purpose.
      */
     @Throws(UnparseableCallException::class)
-    fun parseArguments(methodNameHint: String, parameters: List<Pair<String, Class<*>>>, args: String): Array<Any?> {
+    fun parseArguments(methodNameHint: String, parameters: List<Pair<String, Type>>, args: String): Array<Any?> {
         // If we have parameters, wrap them in {} to allow the Yaml parser to eat them on a single line.
         val parameterString = "{ $args }"
         val tree: JsonNode = om.readTree(parameterString) ?: throw UnparseableCallException(args)
         if (tree.size() > parameters.size) throw UnparseableCallException.TooManyParameters(methodNameHint, args)
         val inOrderParams: List<Any?> = parameters.mapIndexed { _, (argName, argType) ->
             val entry = tree[argName] ?: throw UnparseableCallException.MissingParameter(methodNameHint, argName, args)
+            val entryType = om.typeFactory.constructType(argType)
             try {
-                om.readValue(entry.traverse(om), argType)
+                om.readValue<Any>(entry.traverse(om), entryType)
             } catch (e: Exception) {
                 throw UnparseableCallException.FailedParse(e)
             }

client/jackson/src/main/kotlin/net/corda/client/jackson/internal/JacksonUtils.kt

@@ -3,10 +3,7 @@ package net.corda.client.jackson.internal
 import com.fasterxml.jackson.annotation.JacksonAnnotationsInside
 import com.fasterxml.jackson.core.JsonGenerator
 import com.fasterxml.jackson.core.JsonParser
-import com.fasterxml.jackson.databind.DeserializationContext
+import com.fasterxml.jackson.databind.*
-import com.fasterxml.jackson.databind.JsonDeserializer
-import com.fasterxml.jackson.databind.JsonNode
-import com.fasterxml.jackson.databind.ObjectMapper
 import com.fasterxml.jackson.databind.annotation.JsonSerialize
 import com.fasterxml.jackson.databind.ser.std.ToStringSerializer
 import com.fasterxml.jackson.module.kotlin.convertValue

client/jackson/src/test/kotlin/net/corda/client/jackson/StringToMethodCallParserTest.kt

@@ -3,7 +3,9 @@ package net.corda.client.jackson
 import net.corda.core.crypto.SecureHash
 import org.junit.Assert.assertArrayEquals
 import org.junit.Test
+import java.util.*
 import kotlin.test.assertEquals
+import kotlin.test.assertTrue
 
 class StringToMethodCallParserTest {
     @Suppress("UNUSED")
@@ -13,6 +15,7 @@ class StringToMethodCallParserTest {
         fun twoStrings(a: String, b: String) = a + b
         fun simpleObject(hash: SecureHash.SHA256) = hash.toString()
         fun complexObject(pair: Pair<Int, String>) = pair
+        fun complexNestedObject(pairs: Pair<Int, Deque<Char>>) = pairs
 
         fun overload(a: String) = a
         fun overload(a: String, b: String) = a + b
@@ -38,30 +41,68 @@ class StringToMethodCallParserTest {
         }
     }
 
+    /*
+     * It would be unreasonable to expect "[ A, B, C ]" to deserialise as "Deque<Char>" by default.
+     * Deque is chosen as we still expect it to preserve the order of its elements.
+     */
+    @Test
+    fun complexNestedGenericMethod() {
+        val parser = StringToMethodCallParser(Target::class)
+        val result = parser.parse(Target(), "complexNestedObject pairs: { first: 101, second: [ A, B, C ] }").invoke()
+
+        assertTrue(result is Pair<*,*>)
+        result as Pair<*,*>
+
+        assertEquals(101, result.first)
+
+        assertTrue(result.second is Deque<*>)
+        val deque = result.second as Deque<*>
+        assertArrayEquals(arrayOf('A', 'B', 'C'), deque.toTypedArray())
+    }
+
     @Suppress("UNUSED")
     class ConstructorTarget(val someWord: String, val aDifferentThing: Int) {
         constructor(alternativeWord: String) : this(alternativeWord, 0)
+        constructor(numbers: List<Long>) : this(numbers.map(Long::toString).joinToString("+"), numbers.size)
     }
 
     @Test
     fun ctor1() {
         val clazz = ConstructorTarget::class.java
         val parser = StringToMethodCallParser(clazz)
-        val ctor = clazz.constructors.single { it.parameterCount == 2 }
+        val ctor = clazz.getDeclaredConstructor(String::class.java, Int::class.java)
         val names: List<String> = parser.paramNamesFromConstructor(ctor)
         assertEquals(listOf("someWord", "aDifferentThing"), names)
         val args: Array<Any?> = parser.parseArguments(clazz.name, names.zip(ctor.parameterTypes), "someWord: Blah blah blah, aDifferentThing: 12")
-        assertArrayEquals(args, arrayOf<Any?>("Blah blah blah", 12))
+        assertArrayEquals(arrayOf("Blah blah blah", 12), args)
     }
 
     @Test
     fun ctor2() {
         val clazz = ConstructorTarget::class.java
         val parser = StringToMethodCallParser(clazz)
-        val ctor = clazz.constructors.single { it.parameterCount == 1 }
+        val ctor = clazz.getDeclaredConstructor(String::class.java)
         val names: List<String> = parser.paramNamesFromConstructor(ctor)
         assertEquals(listOf("alternativeWord"), names)
         val args: Array<Any?> = parser.parseArguments(clazz.name, names.zip(ctor.parameterTypes), "alternativeWord: Foo bar!")
-        assertArrayEquals(args, arrayOf<Any?>("Foo bar!"))
+        assertArrayEquals(arrayOf("Foo bar!"), args)
+    }
+
+    @Test
+    fun constructorWithGenericArgs() {
+        val clazz = ConstructorTarget::class.java
+        val ctor = clazz.getDeclaredConstructor(List::class.java)
+        StringToMethodCallParser(clazz).apply {
+            val names = paramNamesFromConstructor(ctor)
+            assertEquals(listOf("numbers"), names)
+
+            val commandLine = "numbers: [ 1, 2, 3 ]"
+
+            val args = parseArguments(clazz.name, names.zip(ctor.parameterTypes), commandLine)
+            assertArrayEquals(arrayOf(listOf(1, 2, 3)), args)
+
+            val trueArgs = parseArguments(clazz.name, names.zip(ctor.genericParameterTypes), commandLine)
+            assertArrayEquals(arrayOf(listOf(1L, 2L, 3L)), trueArgs)
+        }
     }
 }

tools/shell/src/main/kotlin/net/corda/tools/shell/InteractiveShell.kt

@@ -322,16 +322,16 @@ object InteractiveShell {
         for (ctor in clazz.constructors) {
             var paramNamesFromConstructor: List<String>? = null
             fun getPrototype(): List<String> {
-                val argTypes = ctor.parameterTypes.map { it.simpleName }
+                val argTypes = ctor.genericParameterTypes.map { it.typeName }
                 return paramNamesFromConstructor!!.zip(argTypes).map { (name, type) -> "$name: $type" }
             }
 
             try {
                 // Attempt construction with the given arguments.
                 paramNamesFromConstructor = parser.paramNamesFromConstructor(ctor)
-                val args = parser.parseArguments(clazz.name, paramNamesFromConstructor.zip(ctor.parameterTypes), inputData)
+                val args = parser.parseArguments(clazz.name, paramNamesFromConstructor.zip(ctor.genericParameterTypes), inputData)
-                if (args.size != ctor.parameterTypes.size) {
+                if (args.size != ctor.genericParameterTypes.size) {
-                    errors.add("${getPrototype()}: Wrong number of arguments (${args.size} provided, ${ctor.parameterTypes.size} needed)")
+                    errors.add("${getPrototype()}: Wrong number of arguments (${args.size} provided, ${ctor.genericParameterTypes.size} needed)")
                     continue
                 }
                 val flow = ctor.newInstance(*args) as FlowLogic<*>
@@ -345,10 +345,10 @@ object InteractiveShell {
             } catch (e: StringToMethodCallParser.UnparseableCallException.TooManyParameters) {
                 errors.add("${getPrototype()}: too many parameters")
             } catch (e: StringToMethodCallParser.UnparseableCallException.ReflectionDataMissing) {
-                val argTypes = ctor.parameterTypes.map { it.simpleName }
+                val argTypes = ctor.genericParameterTypes.map { it.typeName }
                 errors.add("$argTypes: <constructor missing parameter reflection data>")
             } catch (e: StringToMethodCallParser.UnparseableCallException) {
-                val argTypes = ctor.parameterTypes.map { it.simpleName }
+                val argTypes = ctor.genericParameterTypes.map { it.typeName }
                 errors.add("$argTypes: ${e.message}")
             }
         }

tools/shell/src/test/java/net/corda/tools/shell/InteractiveShellJavaTest.java

@@ -1,10 +1,12 @@
 package net.corda.tools.shell;
 
+import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
 import com.google.common.collect.Lists;
 import kotlin.Pair;
 import net.corda.client.jackson.JacksonSupport;
+import net.corda.client.jackson.internal.ToStringSerialize;
 import net.corda.core.contracts.Amount;
 import net.corda.core.crypto.SecureHash;
 import net.corda.core.flows.FlowException;
@@ -26,18 +28,20 @@ import rx.Observable;
 
 import java.util.*;
 
+import static java.util.stream.Collectors.toList;
 import static org.junit.Assert.assertEquals;
 
 public class InteractiveShellJavaTest {
     private static TestIdentity megaCorp = new TestIdentity(new CordaX500Name("MegaCorp", "London", "GB"));
 
     // should guarantee that FlowA will have synthetic method to access this field
-    private static String synthetic = "synth";
+    private static final String synthetic = "synth";
 
     abstract static class StringFlow extends FlowLogic<String> {
         abstract String getA();
     }
 
+    @SuppressWarnings("unused")
     public static class FlowA extends StringFlow {
 
         private String a;
@@ -68,6 +72,18 @@ public class InteractiveShellJavaTest {
             this(party.getName().toString());
         }
 
+        public FlowA(Integer b, Amount<UserValue> amount) {
+            this(String.format("%d %s", amount.getQuantity() + (b == null ? 0 : b), amount.getToken()));
+        }
+
+        public FlowA(String[] b) {
+            this(String.join("+", b));
+        }
+
+        public FlowA(Amount<UserValue>[] amounts) {
+            this(String.join("++", Arrays.stream(amounts).map(Amount::toString).collect(toList())));
+        }
+
         @Nullable
         @Override
         public ProgressTracker getProgressTracker() {
@@ -75,7 +91,7 @@ public class InteractiveShellJavaTest {
         }
 
         @Override
-        public String call() throws FlowException {
+        public String call() {
             return a;
         }
 
@@ -106,9 +122,7 @@ public class InteractiveShellJavaTest {
             FlowSession session = initiateFlow(party);
 
 
-            Integer integer = session.receive(Integer.class).unwrap((i) -> {
+            Integer integer = session.receive(Integer.class).unwrap((i) -> i);
-                return i;
-            });
 
             return integer.toString();
 
@@ -120,6 +134,24 @@ public class InteractiveShellJavaTest {
         }
     }
 
+    @ToStringSerialize
+    public static class UserValue {
+        private final String label;
+
+        public UserValue(@JsonProperty("label") String label) {
+            this.label = label;
+        }
+
+        public String getLabel() {
+            return label;
+        }
+
+        @Override
+        public String toString() {
+            return label;
+        }
+    }
+
     private InMemoryIdentityService ids = new InMemoryIdentityService(Lists.newArrayList(megaCorp.getIdentity()), InternalTestConstantsKt.getDEV_ROOT_CA().getCertificate());
 
     private ObjectMapper om = JacksonSupport.createInMemoryMapper(ids, new YAMLFactory());
@@ -158,9 +190,35 @@ public class InteractiveShellJavaTest {
     @Test
     public void flowStartWithNestedTypes() throws InteractiveShell.NoApplicableConstructor {
         check(
-                "pair: { first: $100.12, second: df489807f81c8c8829e509e1bcb92e6692b9dd9d624b7456435cb2f51dc82587 }",
+            "pair: { first: $100.12, second: df489807f81c8c8829e509e1bcb92e6692b9dd9d624b7456435cb2f51dc82587 }",
-                "($100.12, df489807f81c8c8829e509e1bcb92e6692b9dd9d624b7456435cb2f51dc82587)",
+            "(100.12 USD, DF489807F81C8C8829E509E1BCB92E6692B9DD9D624B7456435CB2F51DC82587)",
-                FlowA.class);
+            FlowA.class);
+    }
+
+    @Test
+    public void flowStartWithUserAmount() throws InteractiveShell.NoApplicableConstructor {
+        check(
+            "b: 500, amount: { \"quantity\": 10001, \"token\":{ \"label\": \"of value\" } }",
+            "10501 of value",
+            FlowA.class);
+    }
+
+    @Test
+    public void flowStartWithArrayType() throws InteractiveShell.NoApplicableConstructor {
+        check(
+            "b: [ One, Two, Three, Four ]",
+            "One+Two+Three+Four",
+            FlowA.class
+        );
+    }
+
+    @Test
+    public void flowStartWithArrayOfNestedType() throws InteractiveShell.NoApplicableConstructor {
+        check(
+            "amounts: [ { \"quantity\": 10, \"token\": { \"label\": \"(1)\" } }, { \"quantity\": 200, \"token\": { \"label\": \"(2)\" } } ]",
+            "10 (1)++200 (2)",
+            FlowA.class
+        );
     }
 
     @Test(expected = InteractiveShell.NoApplicableConstructor.class)

tools/shell/src/test/kotlin/net/corda/tools/shell/InteractiveShellTest.kt

@@ -1,7 +1,9 @@
 package net.corda.tools.shell
 
+import com.fasterxml.jackson.annotation.JsonProperty
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory
 import net.corda.client.jackson.JacksonSupport
+import net.corda.client.jackson.internal.ToStringSerialize
 import net.corda.core.contracts.Amount
 import net.corda.core.crypto.SecureHash
 import net.corda.core.flows.FlowLogic
@@ -31,6 +33,9 @@ class InteractiveShellTest {
         constructor(amount: Amount<Currency>) : this(amount.toString())
         constructor(pair: Pair<Amount<Currency>, SecureHash.SHA256>) : this(pair.toString())
         constructor(party: Party) : this(party.name.toString())
+        constructor(b: Int?, amount: Amount<UserValue>) : this("${(b ?: 0) + amount.quantity} ${amount.token}")
+        constructor(b: Array<String>) : this(b.joinToString("+"))
+        constructor(amounts: Array<Amount<UserValue>>) : this(amounts.map(Amount<UserValue>::toString).joinToString("++"))
 
         override val progressTracker = ProgressTracker()
         override fun call() = a
@@ -65,8 +70,26 @@ class InteractiveShellTest {
 
     @Test
     fun flowStartWithNestedTypes() = check(
-            "pair: { first: $100.12, second: df489807f81c8c8829e509e1bcb92e6692b9dd9d624b7456435cb2f51dc82587 }",
+        input = "pair: { first: $100.12, second: df489807f81c8c8829e509e1bcb92e6692b9dd9d624b7456435cb2f51dc82587 }",
-            "($100.12, df489807f81c8c8829e509e1bcb92e6692b9dd9d624b7456435cb2f51dc82587)"
+        expected = "(100.12 USD, DF489807F81C8C8829E509E1BCB92E6692B9DD9D624B7456435CB2F51DC82587)"
+    )
+
+    @Test
+    fun flowStartWithArrayType() = check(
+        input = "b: [ One, Two, Three, Four ]",
+        expected = "One+Two+Three+Four"
+    )
+
+    @Test
+    fun flowStartWithUserAmount() = check(
+        input = """b: 500, amount: { "quantity": 10001, "token":{ "label": "of value" } }""",
+        expected = "10501 of value"
+    )
+
+    @Test
+    fun flowStartWithArrayOfNestedTypes() = check(
+        input = """amounts: [ { "quantity": 10, "token": { "label": "(1)" } }, { "quantity": 200, "token": { "label": "(2)" } } ]""",
+        expected = "10 (1)++200 (2)"
     )
 
     @Test(expected = InteractiveShell.NoApplicableConstructor::class)
@@ -80,4 +103,9 @@ class InteractiveShellTest {
 
     @Test
     fun party() = check("party: \"${megaCorp.name}\"", megaCorp.name.toString())
+
+    @ToStringSerialize
+    data class UserValue(@JsonProperty("label") val label: String) {
+        override fun toString() = label
+    }
 }