CORDA-598 Fix notary demo, improve error messages, unduplicate some code (#1597)

* Fix notary demo, improve error messages, unduplicate some code.

* No need to map twice.

* Single quotes around X500 names.

* More specific naming.

* Remove extremely risky refactoring.

node/src/integration-test/kotlin/net/corda/node/services/BFTNotaryServiceTests.kt

@@ -37,8 +37,8 @@ import kotlin.test.assertTrue
 
 class BFTNotaryServiceTests {
     companion object {
-        private val clusterName = CordaX500Name(commonName = BFTNonValidatingNotaryService.type.id, organisation = "BFT", locality = "Zurich", country = "CH")
         private val serviceType = BFTNonValidatingNotaryService.type
+        private val clusterName = CordaX500Name(serviceType.id, "BFT", "Zurich", "CH")
     }
 
     private val mockNet = MockNetwork()
@@ -51,9 +51,8 @@ class BFTNotaryServiceTests {
     private fun bftNotaryCluster(clusterSize: Int, exposeRaces: Boolean = false) {
         Files.deleteIfExists("config" / "currentView") // XXX: Make config object warn if this exists?
         val replicaIds = (0 until clusterSize)
-        val party = ServiceIdentityGenerator.generateToDisk(
+        ServiceIdentityGenerator.generateToDisk(
                 replicaIds.map { mockNet.baseDirectory(mockNet.nextNodeId + it) },
-                serviceType.id,
                 clusterName)
         val bftNotaryService = ServiceInfo(serviceType, clusterName)
         val notaryClusterAddresses = replicaIds.map { NetworkHostAndPort("localhost", 11000 + it * 10) }

node/src/integration-test/kotlin/net/corda/node/services/RaftNotaryServiceTests.kt

@@ -25,7 +25,7 @@ import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
 
 class RaftNotaryServiceTests : NodeBasedTest() {
-    private val notaryName = CordaX500Name(commonName = RaftValidatingNotaryService.type.id, organisation = "RAFT Notary Service", locality = "London", country = "GB")
+    private val notaryName = CordaX500Name(RaftValidatingNotaryService.type.id, "RAFT Notary Service", "London", "GB")
 
     @Test
     fun `detect double spend`() {

node/src/integration-test/kotlin/net/corda/services/messaging/P2PMessagingTest.kt

@@ -32,7 +32,7 @@ import java.util.concurrent.atomic.AtomicInteger
 
 class P2PMessagingTest : NodeBasedTest() {
     private companion object {
-        val DISTRIBUTED_SERVICE_NAME = CordaX500Name(commonName = RaftValidatingNotaryService.type.id, organisation = "DistributedService", locality = "London", country = "GB")
+        val DISTRIBUTED_SERVICE_NAME = CordaX500Name(RaftValidatingNotaryService.type.id, "DistributedService", "London", "GB")
         val SERVICE_2_NAME = CordaX500Name(organisation = "Service 2", locality = "London", country = "GB")
     }
 
@@ -68,7 +68,6 @@ class P2PMessagingTest : NodeBasedTest() {
     fun `communicating with a distributed service which the network map node is part of`() {
         ServiceIdentityGenerator.generateToDisk(
                 listOf(DUMMY_MAP.name, SERVICE_2_NAME).map { baseDirectory(it) },
-                RaftValidatingNotaryService.type.id,
                 DISTRIBUTED_SERVICE_NAME)
 
         val distributedService = ServiceInfo(RaftValidatingNotaryService.type, DISTRIBUTED_SERVICE_NAME)
@@ -240,7 +239,7 @@ class P2PMessagingTest : NodeBasedTest() {
 
     private fun StartedNode<*>.receiveFrom(target: MessageRecipients): CordaFuture<Any> {
         val request = TestRequest(replyTo = network.myAddress)
-        return network.sendRequest<Any>(javaClass.name, request, target)
+        return network.sendRequest(javaClass.name, request, target)
     }
 
     @CordaSerializable

node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt

@@ -433,8 +433,8 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
     protected open fun getNotaryIdentity(): PartyAndCertificate? {
         return advertisedServices.singleOrNull { it.type.isNotary() }?.let {
             it.name?.let {
-                require(it.commonName != null) {"Common name must not be null for notary service, use service type id as common name."}
+                require(it.commonName != null) {"Common name in '$it' must not be null for notary service, use service type id as common name."}
-                require(ServiceType.parse(it.commonName!!).isNotary()) {"Common name for notary service must be the notary service type id."}
+                require(ServiceType.parse(it.commonName!!).isNotary()) {"Common name for notary service in '$it' must be the notary service type id."}
             }
             obtainIdentity(it)
         }
@@ -671,7 +671,7 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
         val nodeCert = certificates[0] as? X509Certificate ?: throw ConfigurationException("Node certificate must be an X.509 certificate")
         val subject = CordaX500Name.build(nodeCert.subjectX500Principal)
         if (subject != name)
-            throw ConfigurationException("The name for $id doesn't match what's in the key store: $name vs $subject")
+            throw ConfigurationException("The name '$name' for $id doesn't match what's in the key store: $subject")
 
         partyKeys += keys
         return PartyAndCertificate(CertificateFactory.getInstance("X509").generateCertPath(certificates))

node/src/main/kotlin/net/corda/node/utilities/ServiceIdentityGenerator.kt

@@ -21,12 +21,11 @@ object ServiceIdentityGenerator {
      *
      * @param dirs List of node directories to place the generated identity and key pairs in.
      * @param serviceId The service id of the distributed service.
-     * @param serviceName The legal name of the distributed service.
+     * @param serviceName The legal name of the distributed service, with service id as CN.
      * @param threshold The threshold for the generated group [CompositeKey].
      */
     // TODO: This needs to write out to the key store, not just files on disk
     fun generateToDisk(dirs: List<Path>,
-                       serviceId: String,
                        serviceName: CordaX500Name,
                        threshold: Int = 1): Party {
         log.trace { "Generating a group identity \"serviceName\" for nodes: ${dirs.joinToString()}" }
@@ -41,8 +40,8 @@ object ServiceIdentityGenerator {
             val serviceKeyCert = X509Utilities.createCertificate(CertificateType.CLIENT_CA, issuer.certificate, issuer.keyPair, serviceName, keyPair.public)
             val compositeKeyCert = X509Utilities.createCertificate(CertificateType.CLIENT_CA, issuer.certificate, issuer.keyPair, serviceName, notaryKey)
             val certPath = Files.createDirectories(dir / "certificates") / "distributedService.jks"
-
             val keystore = loadOrCreateKeyStore(certPath, "cordacadevpass")
+            val serviceId = serviceName.commonName
             keystore.setCertificateEntry("$serviceId-composite-key", compositeKeyCert.cert)
             keystore.setKeyEntry("$serviceId-private-key", keyPair.private, "cordacadevkeypass".toCharArray(), arrayOf(serviceKeyCert.cert, issuer.certificate.cert, rootCert))
             keystore.save(certPath, "cordacadevpass")

samples/notary-demo/src/main/kotlin/net/corda/notarydemo/BFTNotaryCordform.kt

@@ -22,8 +22,9 @@ private val clusterSize = 4 // Minimum size that tolerates a faulty replica.
 private val notaryNames = createNotaryNames(clusterSize)
 
 object BFTNotaryCordform : CordformDefinition("build" / "notary-demo-nodes", notaryNames[0].toString()) {
-    private val clusterName = CordaX500Name(organisation = "BFT", locality = "Zurich", country = "CH")
+    private val serviceType = BFTNonValidatingNotaryService.type
-    private val advertisedService = ServiceInfo(BFTNonValidatingNotaryService.type, clusterName)
+    private val clusterName = CordaX500Name(serviceType.id, "BFT", "Zurich", "CH")
+    private val advertisedService = ServiceInfo(serviceType, clusterName)
 
     init {
         node {
@@ -64,6 +65,6 @@ object BFTNotaryCordform : CordformDefinition("build" / "notary-demo-nodes", not
     }
 
     override fun setup(context: CordformContext) {
-        ServiceIdentityGenerator.generateToDisk(notaryNames.map(CordaX500Name::toString).map(context::baseDirectory), advertisedService.type.id, clusterName, minCorrectReplicas(clusterSize))
+        ServiceIdentityGenerator.generateToDisk(notaryNames.map { context.baseDirectory(it.toString()) }, clusterName, minCorrectReplicas(clusterSize))
     }
 }

samples/notary-demo/src/main/kotlin/net/corda/notarydemo/RaftNotaryCordform.kt

@@ -15,13 +15,14 @@ import net.corda.testing.internal.demorun.*
 
 fun main(args: Array<String>) = RaftNotaryCordform.runNodes()
 
-internal fun createNotaryNames(clusterSize: Int) = (0 until clusterSize).map { CordaX500Name(commonName ="Notary Service $it", organisationUnit = "corda", organisation = "R3 Ltd", locality = "Zurich", state = null, country = "CH") }
+internal fun createNotaryNames(clusterSize: Int) = (0 until clusterSize).map { CordaX500Name("Notary Service $it", "Zurich", "CH") }
 
 private val notaryNames = createNotaryNames(3)
 
 object RaftNotaryCordform : CordformDefinition("build" / "notary-demo-nodes", notaryNames[0].toString()) {
-    private val clusterName = CordaX500Name(organisation = "Raft", locality = "Zurich", country = "CH")
+    private val serviceType = RaftValidatingNotaryService.type
-    private val advertisedService = ServiceInfo(RaftValidatingNotaryService.type, clusterName)
+    private val clusterName = CordaX500Name(serviceType.id, "Raft", "Zurich", "CH")
+    private val advertisedService = ServiceInfo(serviceType, clusterName)
 
     init {
         node {
@@ -61,6 +62,6 @@ object RaftNotaryCordform : CordformDefinition("build" / "notary-demo-nodes", no
     }
 
     override fun setup(context: CordformContext) {
-        ServiceIdentityGenerator.generateToDisk(notaryNames.map(CordaX500Name::toString).map(context::baseDirectory), advertisedService.type.id, clusterName)
+        ServiceIdentityGenerator.generateToDisk(notaryNames.map { context.baseDirectory(it.toString()) }, clusterName)
     }
 }

testing/node-driver/src/main/kotlin/net/corda/testing/driver/Driver.kt

@@ -731,9 +731,9 @@ class DriverDSL(
             rpcUsers: List<User>,
             startInSameProcess: Boolean?
     ): CordaFuture<Pair<Party, List<NodeHandle>>> {
-        val nodeNames = (0 until clusterSize).map { CordaX500Name(organisation = "Notary Service $it", locality = "Zurich", country = "CH") }
+        val nodeNames = (0 until clusterSize).map { CordaX500Name("Notary Service $it", "Zurich", "CH") }
         val paths = nodeNames.map { baseDirectory(it) }
-        ServiceIdentityGenerator.generateToDisk(paths, type.id, notaryName)
+        ServiceIdentityGenerator.generateToDisk(paths, notaryName)
         val advertisedServices = setOf(ServiceInfo(type, notaryName))
         val notaryClusterAddress = portAllocation.nextHostAndPort()
 

testing/node-driver/src/main/kotlin/net/corda/testing/node/NodeBasedTest.kt

@@ -37,7 +37,9 @@ import kotlin.concurrent.thread
  */
 // TODO Some of the logic here duplicates what's in the driver
 abstract class NodeBasedTest : TestDependencyInjectionBase() {
-    val WHITESPACE = "\\s++".toRegex()
+    companion object {
+        private val WHITESPACE = "\\s++".toRegex()
+    }
 
     @Rule
     @JvmField
@@ -133,7 +135,6 @@ abstract class NodeBasedTest : TestDependencyInjectionBase() {
                            serviceType: ServiceType = RaftValidatingNotaryService.type): CordaFuture<List<StartedNode<Node>>> {
         ServiceIdentityGenerator.generateToDisk(
                 (0 until clusterSize).map { baseDirectory(notaryName.copy(organisation = "${notaryName.organisation}-$it")) },
-                serviceType.id,
                 notaryName)
 
         val serviceInfo = ServiceInfo(serviceType, notaryName)