From 68ff33549a1c4405788bf531853cb78f79a96691 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Wed, 22 Mar 2017 15:16:29 +0100 Subject: [PATCH] Address review comments --- .../corda/jackson/StringToMethodCallParser.kt | 9 +++-- docs/source/tutorial-attachments.rst | 2 +- node/src/main/kotlin/net/corda/node/Corda.kt | 2 +- .../kotlin/net/corda/node/InteractiveShell.kt | 33 ++++++++----------- .../kotlin/net/corda/node/internal/Node.kt | 7 ++-- .../persistence/NodeAttachmentService.kt | 8 ++--- .../net/corda/node/shell/base/flow.java | 3 ++ .../net/corda/node/InteractiveShellTest.kt | 12 +++---- .../services/NodeAttachmentStorageTest.kt | 8 +++-- 9 files changed, 42 insertions(+), 42 deletions(-) diff --git a/client/jackson/src/main/kotlin/net/corda/jackson/StringToMethodCallParser.kt b/client/jackson/src/main/kotlin/net/corda/jackson/StringToMethodCallParser.kt index 2008844951..84865c166a 100644 --- a/client/jackson/src/main/kotlin/net/corda/jackson/StringToMethodCallParser.kt +++ b/client/jackson/src/main/kotlin/net/corda/jackson/StringToMethodCallParser.kt @@ -188,15 +188,14 @@ open class StringToMethodCallParser @JvmOverloads constructor( /** Returns a string-to-string map of commands to a string describing available parameter types. */ val availableCommands: Map get() { - return methodMap.map { entry -> - val (name, args) = entry - val argStr = if (args.parameterCount == 0) "" else { + return methodMap.mapValues { entry -> + val (name, args) = entry // TODO: Kotlin 1.1 + if (args.parameterCount == 0) "" else { val paramNames = methodParamNames[name]!! val typeNames = args.parameters.map { it.type.simpleName } val paramTypes = paramNames.zip(typeNames) paramTypes.map { "${it.first}: ${it.second}" }.joinToString(", ") } - Pair(name, argStr) - }.toMap() + } } } \ No newline at end of file diff --git a/docs/source/tutorial-attachments.rst b/docs/source/tutorial-attachments.rst index e734f51857..8570e309ca 100644 --- a/docs/source/tutorial-attachments.rst +++ b/docs/source/tutorial-attachments.rst @@ -32,7 +32,7 @@ To download an attachment, you can do: ``>>> run openAttachment id: AB7FED7663A3F195A59A0F01091932B15C22405CB727A1518418BF53C6E6663A`` -which will then ask you to provide a path to save the file to. To do the same thing programmatically, you would +which will then ask you to provide a path to save the file to. To do the same thing programmatically, you can pass a simple ``InputStream`` or ``SecureHash`` to the ``uploadAttachment``/``openAttachment`` RPCs from a JVM client. diff --git a/node/src/main/kotlin/net/corda/node/Corda.kt b/node/src/main/kotlin/net/corda/node/Corda.kt index f4dd20798a..4a19884012 100644 --- a/node/src/main/kotlin/net/corda/node/Corda.kt +++ b/node/src/main/kotlin/net/corda/node/Corda.kt @@ -132,7 +132,7 @@ fun main(args: Array) { // Don't start the shell if there's no console attached. val runShell = !cmdlineOptions.noLocalShell && System.console() != null - node.startupComplete.thenAccept { + node.startupComplete then { InteractiveShell.startShell(dir, runShell, cmdlineOptions.sshdServer, node) } } failure { diff --git a/node/src/main/kotlin/net/corda/node/InteractiveShell.kt b/node/src/main/kotlin/net/corda/node/InteractiveShell.kt index 4922ae32fd..5b98155540 100644 --- a/node/src/main/kotlin/net/corda/node/InteractiveShell.kt +++ b/node/src/main/kotlin/net/corda/node/InteractiveShell.kt @@ -9,14 +9,11 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLFactory import com.google.common.io.Closeables import com.google.common.util.concurrent.ListenableFuture import com.google.common.util.concurrent.SettableFuture -import net.corda.core.div +import net.corda.core.* import net.corda.core.flows.FlowLogic import net.corda.core.flows.FlowStateMachine import net.corda.core.messaging.CordaRPCOps -import net.corda.core.rootCause -import net.corda.core.then import net.corda.core.utilities.Emoji -import net.corda.core.write import net.corda.jackson.JacksonSupport import net.corda.jackson.StringToMethodCallParser import net.corda.node.internal.Node @@ -82,8 +79,7 @@ object InteractiveShell { val classpathDriver = ClassPathMountFactory(Thread.currentThread().contextClassLoader) val fileDriver = FileMountFactory(Utils.getCurrentDirectory()) - val extraCommandsPath = (dir / "shell-commands").toAbsolutePath() - Files.createDirectories(extraCommandsPath) + val extraCommandsPath = (dir / "shell-commands").toAbsolutePath().createDirectories() val commandsFS = FS.Builder() .register("file", fileDriver) .mount("file:" + extraCommandsPath) @@ -195,7 +191,10 @@ object InteractiveShell { @JvmStatic fun runFlowByNameFragment(nameFragment: String, inputData: String, output: RenderPrintWriter) { val matches = node.flowLogicFactory.flowWhitelist.keys.filter { nameFragment in it } - if (matches.size > 1) { + if (matches.isEmpty()) { + output.println("No matching flow found, run 'flow list' to see your options.", Color.red) + return + } else if (matches.size > 1) { output.println("Ambigous name provided, please be more specific. Your options are:") matches.forEachIndexed { i, s -> output.println("${i+1}. $s", Color.yellow) } return @@ -219,8 +218,6 @@ object InteractiveShell { } catch(e: InterruptedException) { ANSIProgressRenderer.progressTracker = null // TODO: When the flow framework allows us to kill flows mid-flight, do so here. - } catch(e: ExecutionException) { - // It has already been logged by the framework code and printed by the ANSI progress renderer. } } catch(e: NoApplicableConstructor) { output.println("No matching constructor found:", Color.red) @@ -246,7 +243,8 @@ object InteractiveShell { */ @Throws(NoApplicableConstructor::class) fun runFlowFromString(invoke: (FlowLogic<*>) -> FlowStateMachine<*>, - inputData: String, clazz: Class>, + inputData: String, + clazz: Class>, om: ObjectMapper = yamlInputMapper): FlowStateMachine<*> { // 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. @@ -291,7 +289,7 @@ object InteractiveShell { fun runRPCFromString(input: List, out: RenderPrintWriter, context: InvocationContext): Any? { val parser = StringToMethodCallParser(CordaRPCOps::class.java, context.attributes["mapper"] as ObjectMapper) - val cmd = input.joinToString(" ").trim({ it <= ' ' }) + val cmd = input.joinToString(" ").trim { it <= ' ' } if (cmd.toLowerCase().startsWith("startflow")) { // The flow command provides better support and startFlow requires special handling anyway due to // the generic startFlow RPC interface which offers no type information with which to parse the @@ -318,9 +316,9 @@ object InteractiveShell { } catch (e: InterruptedException) { Thread.currentThread().interrupt() } catch (e: ExecutionException) { - throw RuntimeException(e.rootCause) + throw e.rootCause } catch (e: InvocationTargetException) { - throw RuntimeException(e.rootCause) + throw e.rootCause } } } catch (e: StringToMethodCallParser.UnparseableCallException) { @@ -344,15 +342,12 @@ object InteractiveShell { private class PrintingSubscriber(private val printerFun: (Any?) -> String, private val toStream: PrintWriter) : Subscriber() { private var count = 0 - val future = SettableFuture.create()!! + val future: SettableFuture = SettableFuture.create() init { // The future is public and can be completed by something else to indicate we don't wish to follow // anymore (e.g. the user pressing Ctrl-C). - future then { - if (!isUnsubscribed) - unsubscribe() - } + future then { unsubscribe() } } @Synchronized @@ -417,7 +412,7 @@ object InteractiveShell { private val streams = Collections.synchronizedSet(HashSet()) override fun deserialize(p: JsonParser, ctxt: DeserializationContext): InputStream { - val stream = object : FilterInputStream(BufferedInputStream(Files.newInputStream(Paths.get(p.text)))) { + val stream = object : BufferedInputStream(Files.newInputStream(Paths.get(p.text))) { override fun close() { super.close() streams.remove(this) diff --git a/node/src/main/kotlin/net/corda/node/internal/Node.kt b/node/src/main/kotlin/net/corda/node/internal/Node.kt index ef9987e17c..904122317a 100644 --- a/node/src/main/kotlin/net/corda/node/internal/Node.kt +++ b/node/src/main/kotlin/net/corda/node/internal/Node.kt @@ -4,6 +4,7 @@ import com.codahale.metrics.JmxReporter import com.google.common.net.HostAndPort import com.google.common.util.concurrent.Futures import com.google.common.util.concurrent.ListenableFuture +import com.google.common.util.concurrent.SettableFuture import net.corda.core.* import net.corda.core.messaging.RPCOps import net.corda.core.node.NodeVersionInfo @@ -12,7 +13,6 @@ import net.corda.core.node.Version import net.corda.core.node.services.ServiceInfo import net.corda.core.node.services.ServiceType import net.corda.core.node.services.UniquenessProvider -import net.corda.core.success import net.corda.core.utilities.loggerFor import net.corda.node.printBasicNodeInfo import net.corda.node.serialization.NodeClock @@ -33,7 +33,6 @@ import java.io.RandomAccessFile import java.lang.management.ManagementFactory import java.nio.channels.FileLock import java.time.Clock -import java.util.concurrent.CompletableFuture import javax.management.ObjectName import kotlin.concurrent.thread @@ -229,7 +228,7 @@ class Node(override val configuration: FullNodeConfiguration, super.initialiseDatabasePersistence(insideTransaction) } - val startupComplete = CompletableFuture() + val startupComplete: ListenableFuture = SettableFuture.create() override fun start(): Node { alreadyRunningNodeCheck() @@ -254,7 +253,7 @@ class Node(override val configuration: FullNodeConfiguration, build(). start() - startupComplete.complete(Unit) + (startupComplete as SettableFuture).set(Unit) } shutdownThread = thread(start = false) { diff --git a/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt b/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt index ac8c59b5e8..eb79ae2cf3 100644 --- a/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt +++ b/node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt @@ -176,12 +176,12 @@ class NodeAttachmentService(override var storePath: Path, dataSourceProperties: val cursor = jar.nextJarEntry ?: break val entryPath = Paths.get(cursor.name) // Security check to stop zips trying to escape their rightful place. - if (entryPath.isAbsolute || entryPath.normalize() != entryPath || '\\' in cursor.name || cursor.name == "." || cursor.name == "..") - throw IllegalArgumentException("Path is either absolute or non-normalised: $entryPath") + require(!entryPath.isAbsolute) { "Path $entryPath is absolute" } + require(entryPath.normalize() == entryPath) { "Path $entryPath is not normalised" } + require(!('\\' in cursor.name || cursor.name == "." || cursor.name == "..")) { "Bad character in $entryPath" } count++ } - if (count == 0) - throw IllegalArgumentException("Stream is either empty or not a JAR/ZIP") + require(count > 0) { "Stream is either empty or not a JAR/ZIP" } } // Implementations for AcceptsFileUpload diff --git a/node/src/main/resources/net/corda/node/shell/base/flow.java b/node/src/main/resources/net/corda/node/shell/base/flow.java index 49eff218dd..42ccb032df 100644 --- a/node/src/main/resources/net/corda/node/shell/base/flow.java +++ b/node/src/main/resources/net/corda/node/shell/base/flow.java @@ -18,6 +18,9 @@ import static net.corda.node.InteractiveShell.*; ) @Usage("Start a (work)flow on the node. This is how you can change the ledger.") public class flow extends InteractiveShellCommand { + // Note that the class name is deliberately lower case, because we want the command the user types to be + // lower case. CRaSH should ideally lowercase the command names for us, but it doesn't. + @Command public void start( @Usage("The class name of the flow to run, or an unambiguous substring") @Argument String name, diff --git a/node/src/test/kotlin/net/corda/node/InteractiveShellTest.kt b/node/src/test/kotlin/net/corda/node/InteractiveShellTest.kt index b901f342c9..7584026f24 100644 --- a/node/src/test/kotlin/net/corda/node/InteractiveShellTest.kt +++ b/node/src/test/kotlin/net/corda/node/InteractiveShellTest.kt @@ -40,27 +40,27 @@ class InteractiveShellTest { } @Test - fun success() { + fun flowStartSimple() { check("a: Hi there", "Hi there") check("b: 12", "12") check("b: 12, c: Yo", "12Yo") } - @Test fun complex1() = check("amount: £10", "10.00 GBP") + @Test fun flowStartWithComplexTypes() = check("amount: £10", "10.00 GBP") - @Test fun complex2() = check( + @Test fun flowStartWithNestedTypes() = check( "pair: { first: $100.12, second: df489807f81c8c8829e509e1bcb92e6692b9dd9d624b7456435cb2f51dc82587 }", "($100.12, df489807f81c8c8829e509e1bcb92e6692b9dd9d624b7456435cb2f51dc82587)" ) @Test(expected = InteractiveShell.NoApplicableConstructor::class) - fun noArgs() = check("", "") + fun flowStartNoArgs() = check("", "") @Test(expected = InteractiveShell.NoApplicableConstructor::class) - fun missingParam() = check("c: Yo", "") + fun flowMissingParam() = check("c: Yo", "") @Test(expected = InteractiveShell.NoApplicableConstructor::class) - fun tooManyArgs() = check("b: 12, c: Yo, d: Bar", "") + fun flowTooManyParams() = check("b: 12, c: Yo, d: Bar", "") @Test fun party() = check("party: SomeCorp", "SomeCorp") diff --git a/node/src/test/kotlin/net/corda/node/services/NodeAttachmentStorageTest.kt b/node/src/test/kotlin/net/corda/node/services/NodeAttachmentStorageTest.kt index 7215c5603d..403d8d12fe 100644 --- a/node/src/test/kotlin/net/corda/node/services/NodeAttachmentStorageTest.kt +++ b/node/src/test/kotlin/net/corda/node/services/NodeAttachmentStorageTest.kt @@ -126,12 +126,16 @@ class NodeAttachmentStorageTest { } } - @Test(expected = IllegalArgumentException::class) + @Test fun `non jar rejected`() { val storage = NodeAttachmentService(fs.getPath("/"), dataSourceProperties, MetricRegistry()) val path = fs.getPath("notajar") path.writeLines(listOf("Hey", "there!")) - path.read { storage.importAttachment(it) } + path.read { + assertFailsWith("either empty or not a JAR") { + storage.importAttachment(it) + } + } } private var counter = 0