From 0762a61aca127e93d7ce44afff36167b00a8eb36 Mon Sep 17 00:00:00 2001 From: Dan Newton Date: Wed, 1 Aug 2018 07:58:00 +0100 Subject: [PATCH] In AbstractNode check if an initiating flow already has a flow registered to it and throw exception if one has been (#3713) * In AbstractNode check if an initiating flow already has a flow registered to it and throw exception if one has been In AbstractNode.internalRegisterFlowFactory check if a flow factory already exists for a initiating flow If a factory does exist, throw an exception A few tests needed to be fixed due to this change * Reorder test setup to prevent flows from being registered multiple times * Use check instead of if and throws statement in AbstractNode when checking for initiating flow having multiple flows mapped to it Use check instead of if throws statement Change names of methods in tests Improve FlowRegistrationTest to better check if flow has been registered properly * tidy up FlowFrameworkTests and FlowRegistrationTest --- .../net/corda/docs/CustomVaultQueryTest.kt | 1 - .../WorkflowTransactionBuildTutorialTest.kt | 1 - .../services/messaging/MQSecurityTest.kt | 1 - .../net/corda/node/internal/AbstractNode.kt | 3 + .../node/internal/FlowRegistrationTest.kt | 72 ++++++++++++ .../statemachine/FlowFrameworkTests.kt | 103 ++++++++---------- 6 files changed, 120 insertions(+), 61 deletions(-) create mode 100644 node/src/test/kotlin/net/corda/node/internal/FlowRegistrationTest.kt diff --git a/docs/source/example-code/src/test/kotlin/net/corda/docs/CustomVaultQueryTest.kt b/docs/source/example-code/src/test/kotlin/net/corda/docs/CustomVaultQueryTest.kt index 1ac8e1e909..53e92a35c5 100644 --- a/docs/source/example-code/src/test/kotlin/net/corda/docs/CustomVaultQueryTest.kt +++ b/docs/source/example-code/src/test/kotlin/net/corda/docs/CustomVaultQueryTest.kt @@ -42,7 +42,6 @@ class CustomVaultQueryTest { mockNet = MockNetwork(threadPerNode = true, cordappPackages = listOf("net.corda.finance", "net.corda.docs", "com.template")) nodeA = mockNet.createPartyNode() nodeB = mockNet.createPartyNode() - nodeA.registerInitiatedFlow(TopupIssuerFlow.TopupIssuer::class.java) notary = mockNet.defaultNotaryIdentity } diff --git a/docs/source/example-code/src/test/kotlin/net/corda/docs/WorkflowTransactionBuildTutorialTest.kt b/docs/source/example-code/src/test/kotlin/net/corda/docs/WorkflowTransactionBuildTutorialTest.kt index e7d28fb72e..3304f4707a 100644 --- a/docs/source/example-code/src/test/kotlin/net/corda/docs/WorkflowTransactionBuildTutorialTest.kt +++ b/docs/source/example-code/src/test/kotlin/net/corda/docs/WorkflowTransactionBuildTutorialTest.kt @@ -36,7 +36,6 @@ class WorkflowTransactionBuildTutorialTest { mockNet = MockNetwork(threadPerNode = true, cordappPackages = listOf("net.corda.docs")) aliceNode = mockNet.createPartyNode(ALICE_NAME) bobNode = mockNet.createPartyNode(BOB_NAME) - aliceNode.registerInitiatedFlow(RecordCompletionFlow::class.java) alice = aliceNode.services.myInfo.identityFromX500Name(ALICE_NAME) bob = bobNode.services.myInfo.identityFromX500Name(BOB_NAME) } diff --git a/node/src/integration-test/kotlin/net/corda/services/messaging/MQSecurityTest.kt b/node/src/integration-test/kotlin/net/corda/services/messaging/MQSecurityTest.kt index d42949d020..0214cd0f48 100644 --- a/node/src/integration-test/kotlin/net/corda/services/messaging/MQSecurityTest.kt +++ b/node/src/integration-test/kotlin/net/corda/services/messaging/MQSecurityTest.kt @@ -188,7 +188,6 @@ abstract class MQSecurityTest : NodeBasedTest() { protected fun startBobAndCommunicateWithAlice(): Party { val bob = startNode(BOB_NAME) - bob.registerInitiatedFlow(ReceiveFlow::class.java) val bobParty = bob.info.singleIdentity() // Perform a protocol exchange to force the peer queue to be created alice.services.startFlow(SendFlow(bobParty, 0)).resultFuture.getOrThrow() 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 45f9385275..343f25c772 100644 --- a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt +++ b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt @@ -659,6 +659,9 @@ abstract class AbstractNode(val configuration: NodeConfiguration, } else { Observable.empty() } + check(initiatingFlowClass !in flowFactories.keys) { + "$initiatingFlowClass is attempting to register multiple initiated flows" + } flowFactories[initiatingFlowClass] = flowFactory return observable } diff --git a/node/src/test/kotlin/net/corda/node/internal/FlowRegistrationTest.kt b/node/src/test/kotlin/net/corda/node/internal/FlowRegistrationTest.kt new file mode 100644 index 0000000000..e6a9b5fd3e --- /dev/null +++ b/node/src/test/kotlin/net/corda/node/internal/FlowRegistrationTest.kt @@ -0,0 +1,72 @@ +package net.corda.node.internal + +import co.paralleluniverse.fibers.Suspendable +import net.corda.core.flows.FlowLogic +import net.corda.core.flows.FlowSession +import net.corda.core.flows.InitiatedBy +import net.corda.core.flows.InitiatingFlow +import net.corda.core.identity.CordaX500Name +import net.corda.core.identity.Party +import net.corda.core.utilities.unwrap +import net.corda.testing.core.singleIdentity +import net.corda.testing.node.MockNetwork +import net.corda.testing.node.MockNodeParameters +import net.corda.testing.node.StartedMockNode +import org.assertj.core.api.Assertions.assertThatIllegalStateException +import org.junit.After +import org.junit.Before +import org.junit.Test +import kotlin.test.assertNotNull + +class FlowRegistrationTest { + + lateinit var mockNetwork: MockNetwork + lateinit var initiator: StartedMockNode + lateinit var responder: StartedMockNode + + @Before + fun setup() { + // no cordapps scanned so it can be tested in isolation + mockNetwork = MockNetwork(emptyList()) + initiator = mockNetwork.createNode(MockNodeParameters(legalName = CordaX500Name("initiator", "Reading", "GB"))) + responder = mockNetwork.createNode(MockNodeParameters(legalName = CordaX500Name("responder", "Reading", "GB"))) + mockNetwork.runNetwork() + } + + @After + fun tearDown() { + mockNetwork.stopNodes() + } + + @Test + fun `startup fails when two flows initiated by the same flow are registered`() { + // register the same flow twice to invoke the error without causing errors in other tests + responder.registerInitiatedFlow(Responder::class.java) + assertThatIllegalStateException().isThrownBy { responder.registerInitiatedFlow(Responder::class.java) } + } + + @Test + fun `a single initiated flow can be registered without error`() { + responder.registerInitiatedFlow(Responder::class.java) + val result = initiator.startFlow(Initiator(responder.info.singleIdentity())) + mockNetwork.runNetwork() + assertNotNull(result.get()) + } +} + +@InitiatingFlow +class Initiator(val party: Party) : FlowLogic() { + @Suspendable + override fun call(): String { + return initiateFlow(party).sendAndReceive("Hello there").unwrap { it } + } +} + +@InitiatedBy(Initiator::class) +private class Responder(val session: FlowSession) : FlowLogic() { + @Suspendable + override fun call() { + session.receive().unwrap { it } + session.send("What's up") + } +} \ No newline at end of file diff --git a/node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt b/node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt index 0d6dc218d8..948aa59f5d 100644 --- a/node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt +++ b/node/src/test/kotlin/net/corda/node/services/statemachine/FlowFrameworkTests.kt @@ -53,47 +53,41 @@ class FlowFrameworkTests { init { LogHelper.setLevel("+net.corda.flow") } + } - private lateinit var mockNet: InternalMockNetwork - private lateinit var aliceNode: TestStartedNode - private lateinit var bobNode: TestStartedNode - private lateinit var alice: Party - private lateinit var bob: Party - private lateinit var notaryIdentity: Party - private val receivedSessionMessages = ArrayList() + private lateinit var mockNet: InternalMockNetwork + private lateinit var aliceNode: TestStartedNode + private lateinit var bobNode: TestStartedNode + private lateinit var alice: Party + private lateinit var bob: Party + private lateinit var notaryIdentity: Party + private val receivedSessionMessages = ArrayList() - @BeforeClass - @JvmStatic - fun beforeClass() { - mockNet = InternalMockNetwork( - cordappsForAllNodes = cordappsForPackages("net.corda.finance.contracts", "net.corda.testing.contracts"), - servicePeerAllocationStrategy = RoundRobin() - ) + @Before + fun setUpMockNet() { + mockNet = InternalMockNetwork( + cordappsForAllNodes = cordappsForPackages("net.corda.finance.contracts", "net.corda.testing.contracts"), + servicePeerAllocationStrategy = RoundRobin() + ) - aliceNode = mockNet.createNode(InternalMockNodeParameters(legalName = ALICE_NAME)) - bobNode = mockNet.createNode(InternalMockNodeParameters(legalName = BOB_NAME)) + aliceNode = mockNet.createNode(InternalMockNodeParameters(legalName = ALICE_NAME)) + bobNode = mockNet.createNode(InternalMockNodeParameters(legalName = BOB_NAME)) - // Extract identities - alice = aliceNode.info.singleIdentity() - bob = bobNode.info.singleIdentity() - notaryIdentity = mockNet.defaultNotaryIdentity + // Extract identities + alice = aliceNode.info.singleIdentity() + bob = bobNode.info.singleIdentity() + notaryIdentity = mockNet.defaultNotaryIdentity - receivedSessionMessagesObservable().forEach { receivedSessionMessages += it } - } - - private fun receivedSessionMessagesObservable(): Observable { - return mockNet.messagingNetwork.receivedMessages.toSessionTransfers() - } - - @AfterClass @JvmStatic - fun afterClass() { - mockNet.stopNodes() - } + receivedSessionMessagesObservable().forEach { receivedSessionMessages += it } + } + private fun receivedSessionMessagesObservable(): Observable { + return mockNet.messagingNetwork.receivedMessages.toSessionTransfers() } @After fun cleanUp() { + mockNet.stopNodes() receivedSessionMessages.clear() } @@ -474,45 +468,38 @@ class FlowFrameworkTripartyTests { private lateinit var charlie: Party private lateinit var notaryIdentity: Party private val receivedSessionMessages = ArrayList() + } - @BeforeClass - @JvmStatic - fun beforeClass() { - mockNet = InternalMockNetwork( - cordappsForAllNodes = cordappsForPackages("net.corda.finance.contracts", "net.corda.testing.contracts"), - servicePeerAllocationStrategy = RoundRobin() - ) + @Before + fun setUpGlobalMockNet() { + mockNet = InternalMockNetwork( + cordappsForAllNodes = cordappsForPackages("net.corda.finance.contracts", "net.corda.testing.contracts"), + servicePeerAllocationStrategy = RoundRobin() + ) - aliceNode = mockNet.createNode(InternalMockNodeParameters(legalName = ALICE_NAME)) - bobNode = mockNet.createNode(InternalMockNodeParameters(legalName = BOB_NAME)) - charlieNode = mockNet.createNode(InternalMockNodeParameters(legalName = CHARLIE_NAME)) + aliceNode = mockNet.createNode(InternalMockNodeParameters(legalName = ALICE_NAME)) + bobNode = mockNet.createNode(InternalMockNodeParameters(legalName = BOB_NAME)) + charlieNode = mockNet.createNode(InternalMockNodeParameters(legalName = CHARLIE_NAME)) - // Extract identities - alice = aliceNode.info.singleIdentity() - bob = bobNode.info.singleIdentity() - charlie = charlieNode.info.singleIdentity() - notaryIdentity = mockNet.defaultNotaryIdentity - - receivedSessionMessagesObservable().forEach { receivedSessionMessages += it } - } - - @AfterClass @JvmStatic - fun afterClass() { - mockNet.stopNodes() - } - - private fun receivedSessionMessagesObservable(): Observable { - return mockNet.messagingNetwork.receivedMessages.toSessionTransfers() - } + // Extract identities + alice = aliceNode.info.singleIdentity() + bob = bobNode.info.singleIdentity() + charlie = charlieNode.info.singleIdentity() + notaryIdentity = mockNet.defaultNotaryIdentity + receivedSessionMessagesObservable().forEach { receivedSessionMessages += it } } @After fun cleanUp() { + mockNet.stopNodes() receivedSessionMessages.clear() } + private fun receivedSessionMessagesObservable(): Observable { + return mockNet.messagingNetwork.receivedMessages.toSessionTransfers() + } @Test fun `sending to multiple parties`() {