From 7c17bde3c8f3d8b43dda02a0e94af82ea92df5dd Mon Sep 17 00:00:00 2001 From: Andrzej Cichocki Date: Mon, 25 Sep 2017 13:29:02 +0100 Subject: [PATCH] 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. --- .../net/corda/node/services/BFTNotaryServiceTests.kt | 5 ++--- .../net/corda/node/services/RaftNotaryServiceTests.kt | 2 +- .../net/corda/services/messaging/P2PMessagingTest.kt | 5 ++--- .../main/kotlin/net/corda/node/internal/AbstractNode.kt | 6 +++--- .../net/corda/node/utilities/ServiceIdentityGenerator.kt | 5 ++--- .../kotlin/net/corda/notarydemo/BFTNotaryCordform.kt | 7 ++++--- .../kotlin/net/corda/notarydemo/RaftNotaryCordform.kt | 9 +++++---- .../src/main/kotlin/net/corda/testing/driver/Driver.kt | 4 ++-- .../main/kotlin/net/corda/testing/node/NodeBasedTest.kt | 5 +++-- 9 files changed, 24 insertions(+), 24 deletions(-) diff --git a/node/src/integration-test/kotlin/net/corda/node/services/BFTNotaryServiceTests.kt b/node/src/integration-test/kotlin/net/corda/node/services/BFTNotaryServiceTests.kt index 458862123d..ad36386ed9 100644 --- a/node/src/integration-test/kotlin/net/corda/node/services/BFTNotaryServiceTests.kt +++ b/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) } diff --git a/node/src/integration-test/kotlin/net/corda/node/services/RaftNotaryServiceTests.kt b/node/src/integration-test/kotlin/net/corda/node/services/RaftNotaryServiceTests.kt index c70ffb050a..d66d083241 100644 --- a/node/src/integration-test/kotlin/net/corda/node/services/RaftNotaryServiceTests.kt +++ b/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`() { diff --git a/node/src/integration-test/kotlin/net/corda/services/messaging/P2PMessagingTest.kt b/node/src/integration-test/kotlin/net/corda/services/messaging/P2PMessagingTest.kt index cfe5d04854..786e5c0af8 100644 --- a/node/src/integration-test/kotlin/net/corda/services/messaging/P2PMessagingTest.kt +++ b/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 { val request = TestRequest(replyTo = network.myAddress) - return network.sendRequest(javaClass.name, request, target) + return network.sendRequest(javaClass.name, request, target) } @CordaSerializable diff --git a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt index 1b5fdc0afa..d6134078b5 100644 --- a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt +++ b/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(ServiceType.parse(it.commonName!!).isNotary()) {"Common name for notary service must be the notary service type id."} + 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 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)) diff --git a/node/src/main/kotlin/net/corda/node/utilities/ServiceIdentityGenerator.kt b/node/src/main/kotlin/net/corda/node/utilities/ServiceIdentityGenerator.kt index 198a139bd5..e27a3cc8e4 100644 --- a/node/src/main/kotlin/net/corda/node/utilities/ServiceIdentityGenerator.kt +++ b/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, - 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") diff --git a/samples/notary-demo/src/main/kotlin/net/corda/notarydemo/BFTNotaryCordform.kt b/samples/notary-demo/src/main/kotlin/net/corda/notarydemo/BFTNotaryCordform.kt index 0a6032ca92..28b90b1ad2 100644 --- a/samples/notary-demo/src/main/kotlin/net/corda/notarydemo/BFTNotaryCordform.kt +++ b/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 advertisedService = ServiceInfo(BFTNonValidatingNotaryService.type, clusterName) + private val serviceType = BFTNonValidatingNotaryService.type + 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)) } } diff --git a/samples/notary-demo/src/main/kotlin/net/corda/notarydemo/RaftNotaryCordform.kt b/samples/notary-demo/src/main/kotlin/net/corda/notarydemo/RaftNotaryCordform.kt index d2aa76c14d..37ed77fe67 100644 --- a/samples/notary-demo/src/main/kotlin/net/corda/notarydemo/RaftNotaryCordform.kt +++ b/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) = 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 advertisedService = ServiceInfo(RaftValidatingNotaryService.type, clusterName) + private val serviceType = RaftValidatingNotaryService.type + 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) } } diff --git a/testing/node-driver/src/main/kotlin/net/corda/testing/driver/Driver.kt b/testing/node-driver/src/main/kotlin/net/corda/testing/driver/Driver.kt index fa9f2503bf..01ae3fbbf2 100644 --- a/testing/node-driver/src/main/kotlin/net/corda/testing/driver/Driver.kt +++ b/testing/node-driver/src/main/kotlin/net/corda/testing/driver/Driver.kt @@ -731,9 +731,9 @@ class DriverDSL( rpcUsers: List, startInSameProcess: Boolean? ): CordaFuture>> { - 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() diff --git a/testing/node-driver/src/main/kotlin/net/corda/testing/node/NodeBasedTest.kt b/testing/node-driver/src/main/kotlin/net/corda/testing/node/NodeBasedTest.kt index 7cbb549ba8..ce703de814 100644 --- a/testing/node-driver/src/main/kotlin/net/corda/testing/node/NodeBasedTest.kt +++ b/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>> { ServiceIdentityGenerator.generateToDisk( (0 until clusterSize).map { baseDirectory(notaryName.copy(organisation = "${notaryName.organisation}-$it")) }, - serviceType.id, notaryName) val serviceInfo = ServiceInfo(serviceType, notaryName)