Address review comments

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

@@ -188,15 +188,14 @@ open class StringToMethodCallParser<in T : Any> @JvmOverloads constructor(
 
     /** Returns a string-to-string map of commands to a string describing available parameter types. */
     val availableCommands: Map<String, String> get() {
-        return methodMap.map { entry ->
+        return methodMap.mapValues { entry ->
-            val (name, args) = entry
+            val (name, args) = entry   // TODO: Kotlin 1.1
-            val argStr = if (args.parameterCount == 0) "" else {
+            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()
     }
 }

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.
 

node/src/main/kotlin/net/corda/node/Corda.kt

@@ -132,7 +132,7 @@ fun main(args: Array<String>) {
 
             // 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 {

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()
+        val extraCommandsPath = (dir / "shell-commands").toAbsolutePath().createDirectories()
-        Files.createDirectories(extraCommandsPath)
         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<out FlowLogic<*>>,
+                          inputData: String,
+                          clazz: Class<out FlowLogic<*>>,
                           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<String>, out: RenderPrintWriter, context: InvocationContext<out Any>): 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<Any>() {
         private var count = 0
-        val future = SettableFuture.create<Unit>()!!
+        val future: SettableFuture<Unit> = 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 {
+            future then { unsubscribe() }
-                if (!isUnsubscribed)
-                    unsubscribe()
-            }
         }
 
         @Synchronized
@@ -417,7 +412,7 @@ object InteractiveShell {
         private val streams = Collections.synchronizedSet(HashSet<InputStream>())
 
         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)

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<Unit>()
+    val startupComplete: ListenableFuture<Unit> = 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<Unit>).set(Unit)
         }
 
         shutdownThread = thread(start = false) {

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 == "..")
+            require(!entryPath.isAbsolute) { "Path $entryPath is absolute" }
-                throw IllegalArgumentException("Path is either absolute or non-normalised: $entryPath")
+            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)
+        require(count > 0) { "Stream is either empty or not a JAR/ZIP" }
-            throw IllegalArgumentException("Stream is either empty or not a JAR/ZIP")
     }
 
     // Implementations for AcceptsFileUpload

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,

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

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<IllegalArgumentException>("either empty or not a JAR") {
+                storage.importAttachment(it)
+            }
+        }
     }
 
     private var counter = 0