From 659b447362898ca6d42d4a9f712c6b53cfef4d60 Mon Sep 17 00:00:00 2001 From: Clinton Date: Fri, 29 Sep 2017 17:18:06 +0100 Subject: [PATCH] V1 tests and fixes for the ContractConstraints work (#1739) * V1 tests and fixes for the ContractConstraints work * More fixes. --- .../net/corda/core/flows/FlowLogicRef.kt | 14 +--- .../contracts/isolated/IsolatedDummyFlow.kt | 2 +- .../node/services/AttachmentLoadingTests.kt | 83 ++++++++++++------- .../net/corda/node/internal/AbstractNode.kt | 9 +- .../statemachine/FlowLogicRefFactoryImpl.kt | 13 +-- .../statemachine/StateMachineManager.kt | 12 ++- 6 files changed, 73 insertions(+), 60 deletions(-) diff --git a/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt b/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt index 3f9996c081..0b90909180 100644 --- a/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt +++ b/core/src/main/kotlin/net/corda/core/flows/FlowLogicRef.kt @@ -24,16 +24,4 @@ class IllegalFlowLogicException(type: Class<*>, msg: String) : IllegalArgumentEx */ // TODO: align this with the existing [FlowRef] in the bank-side API (probably replace some of the API classes) @CordaSerializable -interface FlowLogicRef - - -/** - * This is just some way to track what attachments need to be in the class loader, but may later include some app - * properties loaded from the attachments. And perhaps the authenticated user for an API call? - */ -@CordaSerializable -data class AppContext(val attachments: List) { - // TODO: build a real [AttachmentsClassLoader] etc - val classLoader: ClassLoader - get() = this.javaClass.classLoader -} +interface FlowLogicRef \ No newline at end of file diff --git a/finance/isolated/src/main/kotlin/net/corda/finance/contracts/isolated/IsolatedDummyFlow.kt b/finance/isolated/src/main/kotlin/net/corda/finance/contracts/isolated/IsolatedDummyFlow.kt index 855c06cea4..97c35c2af3 100644 --- a/finance/isolated/src/main/kotlin/net/corda/finance/contracts/isolated/IsolatedDummyFlow.kt +++ b/finance/isolated/src/main/kotlin/net/corda/finance/contracts/isolated/IsolatedDummyFlow.kt @@ -31,4 +31,4 @@ class IsolatedDummyFlow { stx.verify(serviceHub) } } -} \ No newline at end of file +} diff --git a/node/src/integration-test/kotlin/net/corda/node/services/AttachmentLoadingTests.kt b/node/src/integration-test/kotlin/net/corda/node/services/AttachmentLoadingTests.kt index 353203e557..0d6bc46545 100644 --- a/node/src/integration-test/kotlin/net/corda/node/services/AttachmentLoadingTests.kt +++ b/node/src/integration-test/kotlin/net/corda/node/services/AttachmentLoadingTests.kt @@ -1,10 +1,12 @@ package net.corda.node.services import net.corda.client.rpc.RPCException +import net.corda.core.concurrent.CordaFuture import net.corda.core.contracts.Contract import net.corda.core.contracts.PartyAndReference import net.corda.core.cordapp.CordappProvider import net.corda.core.flows.FlowLogic +import net.corda.core.flows.UnexpectedFlowEndException import net.corda.core.identity.CordaX500Name import net.corda.core.identity.Party import net.corda.core.internal.concurrent.transpose @@ -16,12 +18,18 @@ import net.corda.core.transactions.TransactionBuilder import net.corda.core.utilities.OpaqueBytes import net.corda.core.utilities.getOrThrow import net.corda.core.utilities.loggerFor +import net.corda.node.internal.StartedNode import net.corda.node.internal.cordapp.CordappLoader import net.corda.node.internal.cordapp.CordappProviderImpl +import net.corda.node.services.transactions.SimpleNotaryService import net.corda.nodeapi.User +import net.corda.nodeapi.internal.ServiceInfo import net.corda.testing.DUMMY_BANK_A import net.corda.testing.DUMMY_NOTARY import net.corda.testing.TestDependencyInjectionBase +import net.corda.testing.driver.DriverDSL +import net.corda.testing.driver.DriverDSLExposedInterface +import net.corda.testing.driver.NodeHandle import net.corda.testing.driver.driver import net.corda.testing.node.MockServices import net.corda.testing.resetTestSerialization @@ -30,6 +38,8 @@ import org.junit.Before import org.junit.Test import java.net.URLClassLoader import java.nio.file.Files +import java.sql.Driver +import kotlin.test.assertFails import kotlin.test.assertFailsWith class AttachmentLoadingTests : TestDependencyInjectionBase() { @@ -45,6 +55,13 @@ class AttachmentLoadingTests : TestDependencyInjectionBase() { val logger = loggerFor() val isolatedJAR = AttachmentLoadingTests::class.java.getResource("isolated.jar")!! val ISOLATED_CONTRACT_ID = "net.corda.finance.contracts.isolated.AnotherDummyContract" + + val bankAName = CordaX500Name("BankA", "Zurich", "CH") + val bankBName = CordaX500Name("BankB", "Zurich", "CH") + val notaryName = CordaX500Name("Notary", "Zurich", "CH") + val flowInitiatorClass = + Class.forName("net.corda.finance.contracts.isolated.IsolatedDummyFlow\$Initiator", true, URLClassLoader(arrayOf(isolatedJAR))) + .asSubclass(FlowLogic::class.java) } private lateinit var services: Services @@ -75,40 +92,42 @@ class AttachmentLoadingTests : TestDependencyInjectionBase() { @Test fun `test that attachments retrieved over the network are not used for code`() { driver(initialiseSerialization = false) { - val bankAName = CordaX500Name("BankA", "Zurich", "CH") - val bankBName = CordaX500Name("BankB", "Zurich", "CH") - // Copy the app jar to the first node. The second won't have it. - val path = (baseDirectory(bankAName.toString()) / "plugins").createDirectories() / "isolated.jar" - logger.info("Installing isolated jar to $path") - isolatedJAR.openStream().buffered().use { input -> - Files.newOutputStream(path).buffered().use { output -> - input.copyTo(output) - } - } + installIsolatedCordappTo(bankAName) + val (bankA, bankB, _) = createTwoNodesAndNotary() - val admin = User("admin", "admin", permissions = setOf("ALL")) - val (bankA, bankB) = listOf( - startNode(providedName = bankAName, rpcUsers = listOf(admin)), - startNode(providedName = bankBName, rpcUsers = listOf(admin)) - ).transpose().getOrThrow() // Wait for all nodes to start up. - - val clazz = - Class.forName("net.corda.finance.contracts.isolated.IsolatedDummyFlow\$Initiator", true, URLClassLoader(arrayOf(isolatedJAR))) - .asSubclass(FlowLogic::class.java) - - try { - bankA.rpcClientToNode().start("admin", "admin").use { rpc -> - val proxy = rpc.proxy - val party = proxy.wellKnownPartyFromX500Name(bankBName)!! - - assertFailsWith("net.corda.client.rpc.RPCException: net.corda.finance.contracts.isolated.IsolatedDummyFlow\$Initiator") { - proxy.startFlowDynamic(clazz, party).returnValue.getOrThrow() - } - } - } finally { - bankA.stop() - bankB.stop() + assertFailsWith("Party C=CH,L=Zurich,O=BankB rejected session request: Don't know net.corda.finance.contracts.isolated.IsolatedDummyFlow\$Initiator") { + bankA.rpc.startFlowDynamic(flowInitiatorClass, bankB.nodeInfo.legalIdentities.first()).returnValue.getOrThrow() } } } + + @Test + fun `tests that if the attachment is loaded on both sides already that a flow can run`() { + driver(initialiseSerialization = false) { + installIsolatedCordappTo(bankAName) + installIsolatedCordappTo(bankBName) + val (bankA, bankB, _) = createTwoNodesAndNotary() + bankA.rpc.startFlowDynamic(flowInitiatorClass, bankB.nodeInfo.legalIdentities.first()).returnValue.getOrThrow() + } + } + + private fun DriverDSLExposedInterface.installIsolatedCordappTo(nodeName: CordaX500Name) { + // Copy the app jar to the first node. The second won't have it. + val path = (baseDirectory(nodeName.toString()) / "plugins").createDirectories() / "isolated.jar" + logger.info("Installing isolated jar to $path") + isolatedJAR.openStream().buffered().use { input -> + Files.newOutputStream(path).buffered().use { output -> + input.copyTo(output) + } + } + } + + private fun DriverDSLExposedInterface.createTwoNodesAndNotary(): List { + val adminUser = User("admin", "admin", permissions = setOf("ALL")) + return listOf( + startNode(providedName = bankAName, rpcUsers = listOf(adminUser)), + startNode(providedName = bankBName, rpcUsers = listOf(adminUser)), + startNode(providedName = notaryName, rpcUsers = listOf(adminUser), advertisedServices = setOf(ServiceInfo(SimpleNotaryService.type))) + ).transpose().getOrThrow() // Wait for all nodes to start up. + } } 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 f0711e97a7..9aacb16a83 100644 --- a/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt +++ b/node/src/main/kotlin/net/corda/node/internal/AbstractNode.kt @@ -58,10 +58,7 @@ import net.corda.node.services.persistence.DBTransactionStorage import net.corda.node.services.persistence.NodeAttachmentService import net.corda.node.services.schema.HibernateObserver import net.corda.node.services.schema.NodeSchemaService -import net.corda.node.services.statemachine.FlowStateMachineImpl -import net.corda.node.services.statemachine.StateMachineManager -import net.corda.node.services.statemachine.appName -import net.corda.node.services.statemachine.flowVersionAndInitiatingClass +import net.corda.node.services.statemachine.* import net.corda.node.services.transactions.* import net.corda.node.services.upgrade.ContractUpgradeServiceImpl import net.corda.node.services.vault.NodeVaultService @@ -190,7 +187,8 @@ abstract class AbstractNode(open val configuration: NodeConfiguration, checkpointStorage, serverThread, database, - busyNodeLatch) + busyNodeLatch, + cordappLoader.appClassLoader) smm.tokenizableServices.addAll(tokenizableServices) @@ -213,6 +211,7 @@ abstract class AbstractNode(open val configuration: NodeConfiguration, registerCordappFlows() _services.rpcFlows += cordappProvider.cordapps.flatMap { it.rpcFlows } registerCustomSchemas(cordappProvider.cordapps.flatMap { it.customSchemas }.toSet()) + FlowLogicRefFactoryImpl.classloader = cordappLoader.appClassLoader runOnStop += network::stop StartedNodeImpl(this, _services, info, checkpointStorage, smm, attachments, inNodeNetworkMapService, network, database, rpcOps) diff --git a/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt b/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt index 6d16a94db5..a3c36fa3a4 100644 --- a/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt +++ b/node/src/main/kotlin/net/corda/node/services/statemachine/FlowLogicRefFactoryImpl.kt @@ -2,6 +2,7 @@ package net.corda.node.services.statemachine import net.corda.core.internal.VisibleForTesting import com.google.common.primitives.Primitives +import net.corda.core.cordapp.CordappContext import net.corda.core.flows.* import net.corda.core.serialization.CordaSerializable import net.corda.core.serialization.SingletonSerializeAsToken @@ -17,7 +18,7 @@ import kotlin.reflect.jvm.javaType * The internal concrete implementation of the FlowLogicRef marker interface. */ @CordaSerializable -data class FlowLogicRefImpl internal constructor(val flowLogicClassName: String, val appContext: AppContext, val args: Map) : FlowLogicRef +data class FlowLogicRefImpl internal constructor(val flowLogicClassName: String, val args: Map) : FlowLogicRef /** * A class for conversion to and from [FlowLogic] and [FlowLogicRef] instances. @@ -32,6 +33,9 @@ data class FlowLogicRefImpl internal constructor(val flowLogicClassName: String, * in response to a potential malicious use or buggy update to an app etc. */ object FlowLogicRefFactoryImpl : SingletonSerializeAsToken(), FlowLogicRefFactory { + // TODO: Replace with a per app classloader/cordapp provider/cordapp loader - this will do for now + var classloader = javaClass.classLoader + override fun create(flowClass: Class>, vararg args: Any?): FlowLogicRef { if (!flowClass.isAnnotationPresent(SchedulableFlow::class.java)) { throw IllegalFlowLogicException(flowClass, "because it's not a schedulable flow") @@ -73,17 +77,14 @@ object FlowLogicRefFactoryImpl : SingletonSerializeAsToken(), FlowLogicRefFactor */ @VisibleForTesting internal fun createKotlin(type: Class>, args: Map): FlowLogicRef { - // TODO: we need to capture something about the class loader or "application context" into the ref, - // perhaps as some sort of ThreadLocal style object. For now, just create an empty one. - val appContext = AppContext(emptyList()) // Check we can find a constructor and populate the args to it, but don't call it createConstructor(type, args) - return FlowLogicRefImpl(type.name, appContext, args) + return FlowLogicRefImpl(type.name, args) } fun toFlowLogic(ref: FlowLogicRef): FlowLogic<*> { if (ref !is FlowLogicRefImpl) throw IllegalFlowLogicException(ref.javaClass, "FlowLogicRef was not created via correct FlowLogicRefFactory interface") - val klass = Class.forName(ref.flowLogicClassName, true, ref.appContext.classLoader).asSubclass(FlowLogic::class.java) + val klass = Class.forName(ref.flowLogicClassName, true, classloader).asSubclass(FlowLogic::class.java) return createConstructor(klass, ref.args)() } diff --git a/node/src/main/kotlin/net/corda/node/services/statemachine/StateMachineManager.kt b/node/src/main/kotlin/net/corda/node/services/statemachine/StateMachineManager.kt index 11c3d219bc..28e2c58958 100644 --- a/node/src/main/kotlin/net/corda/node/services/statemachine/StateMachineManager.kt +++ b/node/src/main/kotlin/net/corda/node/services/statemachine/StateMachineManager.kt @@ -77,7 +77,8 @@ class StateMachineManager(val serviceHub: ServiceHubInternal, val checkpointStorage: CheckpointStorage, val executor: AffinityExecutor, val database: CordaPersistence, - private val unfinishedFibers: ReusableLatch = ReusableLatch()) { + private val unfinishedFibers: ReusableLatch = ReusableLatch(), + private val classloader: ClassLoader = javaClass.classLoader) { inner class FiberScheduler : FiberExecutorScheduler("Same thread scheduler", executor) @@ -377,7 +378,12 @@ class StateMachineManager(val serviceHub: ServiceHubInternal, updateCheckpoint(fiber) session to initiatedFlowFactory } catch (e: SessionRejectException) { - logger.warn("${e.logMessage}: $sessionInit") + // TODO: Handle this more gracefully + try { + logger.warn("${e.logMessage}: $sessionInit") + } catch (e: Throwable) { + logger.warn("Problematic session init message during logging", e) + } sendSessionReject(e.rejectMessage) return } catch (e: Exception) { @@ -400,7 +406,7 @@ class StateMachineManager(val serviceHub: ServiceHubInternal, private fun getInitiatedFlowFactory(sessionInit: SessionInit): InitiatedFlowFactory<*> { val initiatingFlowClass = try { - Class.forName(sessionInit.initiatingFlowClass).asSubclass(FlowLogic::class.java) + Class.forName(sessionInit.initiatingFlowClass, true, classloader).asSubclass(FlowLogic::class.java) } catch (e: ClassNotFoundException) { throw SessionRejectException("Don't know ${sessionInit.initiatingFlowClass}") } catch (e: ClassCastException) {