ENT-3322: Improved error reporting in interactive shell when an error occurs after a ctor is matched. (#4969)

* ENT-3322: Improved error reporting when an error occurs in type construction, after the ctor has been found.

* ENT-3322: FlowA in this test violated a restriction of StringToMethodCallParser that parameters with the same name but different types cannot be overloaded.
Fixed up FlowA to not have overloaded operation.

* ENT-3322: Simplified code following review comments.

* ENT-3322: Few updates following PR review comments.
This commit is contained in:
Adel El-Beik 2019-04-12 11:13:53 +01:00 committed by Rick Parker
parent 7e58b7397d
commit 374ae80ab1
4 changed files with 77 additions and 27 deletions

View File

@ -184,6 +184,21 @@ open class StringToMethodCallParser<in T : Any> @JvmOverloads constructor(
throw UnparseableCallException("No overloads of the method matched") // Should be unreachable!
}
/**
* Validates that the argument string matches the constructor parameters, i.e. this is a matching constructor
* for the argument string. Exception is thrown if not a match
*
* @param methodNameHint A name that will be used in exceptions if thrown; not used for any other purpose.
* @throws UnparseableCallException If no match is found between constructor parameters and passed string.
*/
@Throws(UnparseableCallException::class)
fun validateIsMatchingCtor(methodNameHint: String, parameters: List<Pair<String, Type>>, args: String) {
val tree = createJsonTreeAndValidate(methodNameHint, parameters, args)
val inOrderParams: List<Any?> = parameters.mapIndexed { _, (argName, argType) ->
tree[argName] ?: throw UnparseableCallException.MissingParameter(methodNameHint, argName, args)
}
}
/**
* Parses only the arguments string given the info about parameter names and types.
*
@ -191,10 +206,7 @@ open class StringToMethodCallParser<in T : Any> @JvmOverloads constructor(
*/
@Throws(UnparseableCallException::class)
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 tree = createJsonTreeAndValidate(methodNameHint, parameters, 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)
@ -213,6 +225,14 @@ open class StringToMethodCallParser<in T : Any> @JvmOverloads constructor(
return inOrderParams.toTypedArray()
}
private fun createJsonTreeAndValidate(methodNameHint: String, parameters: List<Pair<String, Type>>, args: String) : JsonNode {
// 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)
return tree
}
/** Returns a string-to-string map of commands to a string describing available parameter types. */
val availableCommands: Map<String, String>
get() {

View File

@ -389,41 +389,60 @@ object InteractiveShell {
inputData: String,
clazz: Class<out FlowLogic<T>>,
om: ObjectMapper): FlowProgressHandle<T> {
// For each constructor, attempt to parse the input data as a method call. Use the first that succeeds,
// and keep track of the reasons we failed so we can print them out if no constructors are usable.
val parser = StringToMethodCallParser(clazz, om)
val errors = ArrayList<String>()
val errors = ArrayList<String>()
val parser = StringToMethodCallParser(clazz, om)
val nameTypeList = getMatchingConstructorParamsAndTypes(parser, inputData, clazz)
try {
val args = parser.parseArguments(clazz.name, nameTypeList, inputData)
return invoke(clazz, args)
} catch (e: StringToMethodCallParser.UnparseableCallException.ReflectionDataMissing) {
val argTypes = nameTypeList.map { (_, type) -> type }
errors.add("$argTypes: <constructor missing parameter reflection data>")
} catch (e: StringToMethodCallParser.UnparseableCallException) {
val argTypes = nameTypeList.map { (_, type) -> type }
errors.add("$argTypes: ${e.message}")
}
throw NoApplicableConstructor(errors)
}
private fun <T> getMatchingConstructorParamsAndTypes(parser: StringToMethodCallParser<FlowLogic<T>>,
inputData: String,
clazz: Class<out FlowLogic<T>>) : List<Pair<String, Type>> {
val errors = ArrayList<String>()
val classPackage = clazz.packageName
for (ctor in clazz.constructors) {
var paramNamesFromConstructor: List<String>? = null
lateinit var paramNamesFromConstructor: List<String>
for (ctor in clazz.constructors) { // Attempt construction with the given arguments.
fun getPrototype(): List<String> {
val argTypes = ctor.genericParameterTypes.map { it: Type ->
val argTypes = ctor.genericParameterTypes.map {
// If the type name is in the net.corda.core or java namespaces, chop off the package name
// because these hierarchies don't have (m)any ambiguous names and the extra detail is just noise.
maybeAbbreviateGenericType(it, classPackage)
}
return paramNamesFromConstructor!!.zip(argTypes).map { (name, type) -> "$name: $type" }
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.genericParameterTypes), inputData)
if (args.size != ctor.genericParameterTypes.size) {
errors.add("${getPrototype()}: Wrong number of arguments (${args.size} provided, ${ctor.genericParameterTypes.size} needed)")
continue
}
return invoke(clazz, args)
} catch (e: StringToMethodCallParser.UnparseableCallException.MissingParameter) {
val nameTypeList = paramNamesFromConstructor.zip(ctor.genericParameterTypes)
parser.validateIsMatchingCtor(clazz.name, nameTypeList, inputData)
return nameTypeList
}
catch (e: StringToMethodCallParser.UnparseableCallException.MissingParameter) {
errors.add("${getPrototype()}: missing parameter ${e.paramName}")
} catch (e: StringToMethodCallParser.UnparseableCallException.TooManyParameters) {
}
catch (e: StringToMethodCallParser.UnparseableCallException.TooManyParameters) {
errors.add("${getPrototype()}: too many parameters")
} catch (e: StringToMethodCallParser.UnparseableCallException.ReflectionDataMissing) {
}
catch (e: StringToMethodCallParser.UnparseableCallException.ReflectionDataMissing) {
val argTypes = ctor.genericParameterTypes.map { it.typeName }
errors.add("$argTypes: <constructor missing parameter reflection data>")
} catch (e: StringToMethodCallParser.UnparseableCallException) {
}
catch (e: StringToMethodCallParser.UnparseableCallException) {
val argTypes = ctor.genericParameterTypes.map { it.typeName }
errors.add("$argTypes: ${e.message}")
}

View File

@ -30,6 +30,7 @@ import java.util.*;
import static java.util.stream.Collectors.toList;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
public class InteractiveShellJavaTest {
private static TestIdentity megaCorp = new TestIdentity(new CordaX500Name("MegaCorp", "London", "GB"));
@ -259,4 +260,14 @@ public class InteractiveShellJavaTest {
assertEquals("[amount: Amount<Currency>, abc: int]: missing parameter abc", e.getErrors().get(1));
}
}
@Test
public void flowStartWithUnknownParty() throws InteractiveShell.NoApplicableConstructor {
try {
check("party: nonexistent", "", FlowA.class);
} catch (InteractiveShell.NoApplicableConstructor e) {
assertTrue(e.getErrors().get(0).contains("No matching Party found"));
assertEquals(1, e.getErrors().size());
}
}
}

View File

@ -154,7 +154,7 @@ class InteractiveShellTest {
@Test
fun flowStartWithArrayType() = check(
input = "b: [ One, Two, Three, Four ]",
input = "c: [ One, Two, Three, Four ]",
expected = "One+Two+Three+Four"
)
@ -174,7 +174,7 @@ class InteractiveShellTest {
fun flowStartNoArgs() = check("", "")
@Test(expected = InteractiveShell.NoApplicableConstructor::class)
fun flowMissingParam() = check("c: Yo", "")
fun flowMissingParam() = check("d: Yo", "")
@Test(expected = InteractiveShell.NoApplicableConstructor::class)
fun flowTooManyParams() = check("b: 12, c: Yo, d: Bar", "")
@ -190,7 +190,7 @@ class InteractiveShellTest {
"[pair: Pair<Amount<Currency>, SecureHash.SHA256>]: missing parameter pair",
"[party: Party]: missing parameter party",
"[b: Integer, amount: Amount<UserValue>]: missing parameter b",
"[b: String[]]: missing parameter b",
"[c: String[]]: missing parameter c",
"[b: Integer, c: String]: missing parameter b",
"[a: String]: missing parameter a",
"[b: Integer]: missing parameter b"
@ -275,7 +275,7 @@ class FlowA(val a: String) : FlowLogic<String>() {
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(c: Array<String>) : this(c.joinToString("+"))
constructor(amounts: Array<Amount<UserValue>>) : this(amounts.joinToString("++", transform = Amount<UserValue>::toString))
override val progressTracker = ProgressTracker()