Refactor CorDapp loading into a separate module (#1493)

CordappLoader - Added a new cordapp loader class to encapsulate loading of cordapp classes. Added a utils for classloading. Moved a lot of code out of abstract node into the new loader.

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

@@ -4,8 +4,6 @@ import com.codahale.metrics.MetricRegistry
 import com.google.common.collect.Lists
 import com.google.common.collect.MutableClassToInstanceMap
 import com.google.common.util.concurrent.MoreExecutors
-import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner
-import io.github.lukehutch.fastclasspathscanner.scanner.ScanResult
 import net.corda.core.concurrent.CordaFuture
 import net.corda.core.crypto.*
 import net.corda.core.flows.*
@@ -32,6 +30,8 @@ import net.corda.core.transactions.SignedTransaction
 import net.corda.core.utilities.NetworkHostAndPort
 import net.corda.core.utilities.cert
 import net.corda.core.utilities.debug
+import net.corda.node.internal.classloading.CordappLoader
+import net.corda.node.internal.classloading.requireAnnotation
 import net.corda.node.services.NotaryChangeHandler
 import net.corda.node.services.NotifyTransactionHandler
 import net.corda.node.services.TransactionKeyHandler
@@ -72,9 +72,6 @@ import org.slf4j.Logger
 import rx.Observable
 import java.io.IOException
 import java.lang.reflect.InvocationTargetException
-import java.lang.reflect.Modifier.*
-import java.net.JarURLConnection
-import java.net.URI
 import java.nio.file.Path
 import java.nio.file.Paths
 import java.security.KeyPair
@@ -87,10 +84,7 @@ import java.util.*
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.ExecutorService
 import java.util.concurrent.TimeUnit.SECONDS
-import java.util.stream.Collectors.toList
 import kotlin.collections.ArrayList
-import kotlin.collections.component1
-import kotlin.collections.component2
 import kotlin.collections.set
 import kotlin.reflect.KClass
 import net.corda.core.crypto.generateKeyPair as cryptoGenerateKeyPair
@@ -109,7 +103,6 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
                             val advertisedServices: Set<ServiceInfo>,
                             val platformClock: Clock,
                             @VisibleForTesting val busyNodeLatch: ReusableLatch = ReusableLatch()) : SingletonSerializeAsToken() {
-
     // TODO: Persist this, as well as whether the node is registered.
     /**
      * Sequence number of changes sent to the network map service, when registering/de-registering this node.
@@ -160,8 +153,18 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
         ServiceLoader.load(CordaPluginRegistry::class.java).toList()
     }
 
+    val cordappLoader: CordappLoader by lazy {
+        if (System.getProperty("net.corda.node.cordapp.scan.package") != null) {
+            check(configuration.devMode) { "Package scanning can only occur in dev mode" }
+            CordappLoader.createDevMode(System.getProperty("net.corda.node.cordapp.scan.package"))
+        } else {
+            CordappLoader.createDefault(configuration.baseDirectory)
+        }
+    }
+
     /** Set to true once [start] has been successfully called. */
-    @Volatile var started = false
+    @Volatile
+    var started = false
         private set
 
     /** The implementation of the [CordaRPCOps] interface used by this node. */
@@ -208,12 +211,9 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
             startMessagingService(rpcOps)
             installCoreFlows()
 
-            val scanResult = scanCordapps()
+            installCordaServices()
-            if (scanResult != null) {
+            registerCordappFlows()
-                installCordaServices(scanResult)
+            _services.rpcFlows += cordappLoader.findRPCFlows()
-                registerInitiatedFlows(scanResult)
-                findRPCFlows(scanResult)
-            }
 
             runOnStop += network::stop
         }
@@ -230,28 +230,8 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
 
     private class ServiceInstantiationException(cause: Throwable?) : Exception(cause)
 
-    private fun installCordaServices(scanResult: ScanResult) {
+    private fun installCordaServices() {
-        fun getServiceType(clazz: Class<*>): ServiceType? {
+        cordappLoader.findServices(info).forEach {
-            return try {
-                clazz.getField("type").get(null) as ServiceType
-            } catch (e: NoSuchFieldException) {
-                log.warn("${clazz.name} does not have a type field, optimistically proceeding with install.")
-                null
-            }
-        }
-
-        scanResult.getClassesWithAnnotation(SerializeAsToken::class, CordaService::class)
-                .filter {
-                    val serviceType = getServiceType(it)
-                    if (serviceType != null && info.serviceIdentities(serviceType).isEmpty()) {
-                        log.debug { "Ignoring ${it.name} as a Corda service since $serviceType is not one of our " +
-                                "advertised services" }
-                        false
-                    } else {
-                        true
-                    }
-                }
-                .forEach {
             try {
                 installCordaService(it)
             } catch (e: NoSuchMethodException) {
@@ -292,24 +272,8 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
         installCoreFlow(NotaryFlow.Client::class, service::createServiceFlow)
     }
 
-    private inline fun <reified A : Annotation> Class<*>.requireAnnotation(): A {
+    private fun registerCordappFlows() {
-        return requireNotNull(getDeclaredAnnotation(A::class.java)) { "$name needs to be annotated with ${A::class.java.name}" }
+        cordappLoader.findInitiatedFlows()
-    }
-
-    private fun registerInitiatedFlows(scanResult: ScanResult) {
-        scanResult
-                .getClassesWithAnnotation(FlowLogic::class, InitiatedBy::class)
-                // First group by the initiating flow class in case there are multiple mappings
-                .groupBy { it.requireAnnotation<InitiatedBy>().value.java }
-                .map { (initiatingFlow, initiatedFlows) ->
-                    val sorted = initiatedFlows.sortedWith(FlowTypeHierarchyComparator(initiatingFlow))
-                    if (sorted.size > 1) {
-                        log.warn("${initiatingFlow.name} has been specified as the initiating flow by multiple flows " +
-                                "in the same type hierarchy: ${sorted.joinToString { it.name }}. Choosing the most " +
-                                "specific sub-type for registration: ${sorted[0].name}.")
-                    }
-                    sorted[0]
-                }
                 .forEach {
                     try {
                         registerInitiatedFlowInternal(it, track = false)
@@ -322,21 +286,6 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
                 }
     }
 
-    private class FlowTypeHierarchyComparator(val initiatingFlow: Class<out FlowLogic<*>>) : Comparator<Class<out FlowLogic<*>>> {
-        override fun compare(o1: Class<out FlowLogic<*>>, o2: Class<out FlowLogic<*>>): Int {
-            return if (o1 == o2) {
-                0
-            } else if (o1.isAssignableFrom(o2)) {
-                1
-            } else if (o2.isAssignableFrom(o1)) {
-                -1
-            } else {
-                throw IllegalArgumentException("${initiatingFlow.name} has been specified as the initiating flow by " +
-                        "both ${o1.name} and ${o2.name}")
-            }
-        }
-    }
-
     /**
      * Use this method to register your initiated flows in your tests. This is automatically done by the node when it
      * starts up for all [FlowLogic] classes it finds which are annotated with [InitiatedBy].
@@ -373,19 +322,6 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
         return observable
     }
 
-    private fun findRPCFlows(scanResult: ScanResult) {
-        fun Class<out FlowLogic<*>>.isUserInvokable(): Boolean {
-            return isPublic(modifiers) && !isLocalClass && !isAnonymousClass && (!isMemberClass || isStatic(modifiers))
-        }
-
-        _services.rpcFlows += scanResult
-                .getClassesWithAnnotation(FlowLogic::class, StartableByRPC::class)
-                .filter { it.isUserInvokable() } +
-                    // Add any core flows here
-                    listOf(
-                            ContractUpgradeFlow.Initiator::class.java)
-    }
-
     /**
      * Installs a flow that's core to the Corda platform. Unlike CorDapp flows which are versioned individually using
      * [InitiatingFlow.version], core flows have the same version as the node's platform version. To cater for backwards
@@ -428,58 +364,6 @@ abstract class AbstractNode(open val configuration: NodeConfiguration,
 
     protected open fun makeTransactionStorage(): WritableTransactionStorage = DBTransactionStorage()
 
-    private fun scanCordapps(): ScanResult? {
-        val scanPackage = System.getProperty("net.corda.node.cordapp.scan.package")
-        val paths = if (scanPackage != null) {
-            // Rather than looking in the plugins directory, figure out the classpath for the given package and scan that
-            // instead. This is used in tests where we avoid having to package stuff up in jars and then having to move
-            // them to the plugins directory for each node.
-            check(configuration.devMode) { "Package scanning can only occur in dev mode" }
-            val resource = scanPackage.replace('.', '/')
-            javaClass.classLoader.getResources(resource)
-                    .asSequence()
-                    .map {
-                        val uri = if (it.protocol == "jar") {
-                            (it.openConnection() as JarURLConnection).jarFileURL.toURI()
-                        } else {
-                            URI(it.toExternalForm().removeSuffix(resource))
-                        }
-                        Paths.get(uri)
-                    }
-                    .toList()
-        } else {
-            val pluginsDir = configuration.baseDirectory / "plugins"
-            if (!pluginsDir.exists()) return null
-            pluginsDir.list {
-                it.filter { it.isRegularFile() && it.toString().endsWith(".jar") }.collect(toList())
-            }
-        }
-
-        log.info("Scanning CorDapps in $paths")
-
-        // This will only scan the plugin jars and nothing else
-        return if (paths.isNotEmpty()) FastClasspathScanner().overrideClasspath(paths).scan() else null
-    }
-
-    private fun <T : Any> ScanResult.getClassesWithAnnotation(type: KClass<T>, annotation: KClass<out Annotation>): List<Class<out T>> {
-        fun loadClass(className: String): Class<out T>? {
-            return try {
-                // TODO Make sure this is loaded by the correct class loader
-                Class.forName(className, false, javaClass.classLoader).asSubclass(type.java)
-            } catch (e: ClassCastException) {
-                log.warn("As $className is annotated with ${annotation.qualifiedName} it must be a sub-type of ${type.java.name}")
-                null
-            } catch (e: Exception) {
-                log.warn("Unable to load class $className", e)
-                null
-            }
-        }
-
-        return getNamesOfClassesWithAnnotation(annotation.java)
-                .mapNotNull { loadClass(it) }
-                .filterNot { isAbstract(it.modifiers) }
-    }
-
     private fun makeVaultObservers() {
         VaultSoftLockManager(services.vaultService, smm)
         ScheduledActivityObserver(services)

node/src/main/kotlin/net/corda/node/internal/classloading/CordappLoader.kt

@@ -0,0 +1,164 @@
+package net.corda.node.internal.classloading
+
+import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner
+import io.github.lukehutch.fastclasspathscanner.scanner.ScanResult
+import net.corda.core.flows.ContractUpgradeFlow
+import net.corda.core.flows.FlowLogic
+import net.corda.core.flows.InitiatedBy
+import net.corda.core.flows.StartableByRPC
+import net.corda.core.internal.div
+import net.corda.core.internal.exists
+import net.corda.core.internal.isRegularFile
+import net.corda.core.internal.list
+import net.corda.core.node.NodeInfo
+import net.corda.core.node.services.CordaService
+import net.corda.core.node.services.ServiceType
+import net.corda.core.serialization.SerializeAsToken
+import net.corda.core.utilities.debug
+import net.corda.core.utilities.loggerFor
+import java.lang.reflect.Modifier
+import java.net.JarURLConnection
+import java.net.URI
+import java.nio.file.Path
+import java.nio.file.Paths
+import java.util.*
+import java.util.stream.Collectors
+import kotlin.reflect.KClass
+
+/**
+ * Handles CorDapp loading and classpath scanning
+ */
+class CordappLoader private constructor (val cordappClassPath: List<Path>) {
+    val appClassLoader: ClassLoader = javaClass.classLoader
+    val scanResult = scanCordapps()
+
+    companion object {
+        private val logger = loggerFor<CordappLoader>()
+
+        /**
+         * Creates the default CordappLoader intended to be used in non-dev or non-test environments.
+         *
+         * @param   basedir The directory that this node is running in. Will use this to resolve the plugins directory
+         *                  for classpath scanning.
+         */
+        fun createDefault(baseDir: Path): CordappLoader {
+            val pluginsDir = baseDir / "plugins"
+            return CordappLoader(if (!pluginsDir.exists()) emptyList<Path>() else pluginsDir.list {
+                it.filter { it.isRegularFile() && it.toString().endsWith(".jar") }.collect(Collectors.toList())
+            })
+        }
+
+        /**
+         * Creates the dev mode CordappLoader intended to only be used in dev or test environments.
+         *
+         * @param   scanPackage Resolves the JARs that contain scanPackage and use them as the source for
+         *                      the classpath scanning.
+         */
+        fun createDevMode(scanPackage: String): CordappLoader {
+            val resource = scanPackage.replace('.', '/')
+            val paths = javaClass.classLoader.getResources(resource)
+                    .asSequence()
+                    .map {
+                        val uri = if (it.protocol == "jar") {
+                            (it.openConnection() as JarURLConnection).jarFileURL.toURI()
+                        } else {
+                            URI(it.toExternalForm().removeSuffix(resource))
+                        }
+                        Paths.get(uri)
+                    }
+                    .toList()
+            return CordappLoader(paths)
+        }
+    }
+
+    fun findServices(info: NodeInfo): List<Class<out SerializeAsToken>> {
+        fun getServiceType(clazz: Class<*>): ServiceType? {
+            return try {
+                clazz.getField("type").get(null) as ServiceType
+            } catch (e: NoSuchFieldException) {
+                logger.warn("${clazz.name} does not have a type field, optimistically proceeding with install.")
+                null
+            }
+        }
+
+        return scanResult?.getClassesWithAnnotation(SerializeAsToken::class, CordaService::class)
+                ?.filter {
+                    val serviceType = getServiceType(it)
+                    if (serviceType != null && info.serviceIdentities(serviceType).isEmpty()) {
+                        logger.debug {
+                            "Ignoring ${it.name} as a Corda service since $serviceType is not one of our " +
+                                    "advertised services"
+                        }
+                        false
+                    } else {
+                        true
+                    }
+                } ?: emptyList<Class<SerializeAsToken>>()
+    }
+
+    fun findInitiatedFlows(): List<Class<out FlowLogic<*>>> {
+        return scanResult?.getClassesWithAnnotation(FlowLogic::class, InitiatedBy::class)
+                // First group by the initiating flow class in case there are multiple mappings
+                ?.groupBy { it.requireAnnotation<InitiatedBy>().value.java }
+                ?.map { (initiatingFlow, initiatedFlows) ->
+                    val sorted = initiatedFlows.sortedWith(FlowTypeHierarchyComparator(initiatingFlow))
+                    if (sorted.size > 1) {
+                        logger.warn("${initiatingFlow.name} has been specified as the inititating flow by multiple flows " +
+                                "in the same type hierarchy: ${sorted.joinToString { it.name }}. Choosing the most " +
+                                "specific sub-type for registration: ${sorted[0].name}.")
+                    }
+                    sorted[0]
+                } ?: emptyList<Class<out FlowLogic<*>>>()
+    }
+
+    fun findRPCFlows(): List<Class<out FlowLogic<*>>> {
+        fun Class<out FlowLogic<*>>.isUserInvokable(): Boolean {
+            return Modifier.isPublic(modifiers) && !isLocalClass && !isAnonymousClass && (!isMemberClass || Modifier.isStatic(modifiers))
+        }
+
+        val found = scanResult?.getClassesWithAnnotation(FlowLogic::class, StartableByRPC::class)?.filter { it.isUserInvokable() } ?: emptyList<Class<out FlowLogic<*>>>()
+        val coreFlows = listOf(ContractUpgradeFlow.Initiator::class.java)
+        return found + coreFlows
+    }
+
+    private fun scanCordapps(): ScanResult? {
+        logger.info("Scanning CorDapps in $cordappClassPath")
+        return if (cordappClassPath.isNotEmpty())
+            FastClasspathScanner().addClassLoader(appClassLoader).overrideClasspath(cordappClassPath).scan()
+        else
+            null
+    }
+
+    private class FlowTypeHierarchyComparator(val initiatingFlow: Class<out FlowLogic<*>>) : Comparator<Class<out FlowLogic<*>>> {
+        override fun compare(o1: Class<out FlowLogic<*>>, o2: Class<out FlowLogic<*>>): Int {
+            return if (o1 == o2) {
+                0
+            } else if (o1.isAssignableFrom(o2)) {
+                1
+            } else if (o2.isAssignableFrom(o1)) {
+                -1
+            } else {
+                throw IllegalArgumentException("${initiatingFlow.name} has been specified as the initiating flow by " +
+                        "both ${o1.name} and ${o2.name}")
+            }
+        }
+    }
+
+    private fun <T : Any> ScanResult.getClassesWithAnnotation(type: KClass<T>, annotation: KClass<out Annotation>): List<Class<out T>> {
+        fun loadClass(className: String): Class<out T>? {
+            return try {
+                appClassLoader.loadClass(className) as Class<T>
+            } catch (e: ClassCastException) {
+                logger.warn("As $className is annotated with ${annotation.qualifiedName} it must be a sub-type of ${type.java.name}")
+                null
+            } catch (e: Exception) {
+                logger.warn("Unable to load class $className", e)
+                null
+            }
+        }
+
+        return getNamesOfClassesWithAnnotation(annotation.java)
+                .mapNotNull { loadClass(it) }
+                .filterNot { Modifier.isAbstract(it.modifiers) }
+    }
+}

node/src/main/kotlin/net/corda/node/internal/classloading/Utils.kt

@@ -0,0 +1,7 @@
+@file:JvmName("Utils")
+
+package net.corda.node.internal.classloading
+
+inline fun <reified A : Annotation> Class<*>.requireAnnotation(): A {
+    return requireNotNull(getDeclaredAnnotation(A::class.java)) { "$name needs to be annotated with ${A::class.java.name}" }
+}

node/src/test/kotlin/net/corda/node/classloading/CordappLoaderTest.kt

@@ -0,0 +1,38 @@
+package net.corda.node.classloading
+
+import net.corda.core.flows.FlowLogic
+import net.corda.core.flows.InitiatedBy
+import net.corda.node.internal.classloading.CordappLoader
+import org.junit.After
+import org.junit.Assert
+import org.junit.Before
+import org.junit.Test
+import java.net.URLClassLoader
+import java.nio.file.Path
+import java.nio.file.Paths
+
+class DummyFlow : FlowLogic<Unit>() {
+    override fun call() { }
+}
+
+@InitiatedBy(DummyFlow::class)
+class LoaderTestFlow : FlowLogic<Unit>() {
+    override fun call() { }
+}
+
+class CordappLoaderTest {
+    @Test
+    fun `test that classes that aren't in cordapps aren't loaded`() {
+        // Basedir will not be a corda node directory so the dummy flow shouldn't be recognised as a part of a cordapp
+        val loader = CordappLoader.createDefault(Paths.get("."))
+        Assert.assertNull(loader.findInitiatedFlows().find { it == LoaderTestFlow::class })
+    }
+
+    @Test
+    fun `test that classes that are in a cordapp are loaded`() {
+        val loader = CordappLoader.createDevMode("net.corda.node.classloading")
+        val initiatedFlows = loader.findInitiatedFlows()
+        val expectedClass = loader.appClassLoader.loadClass("net.corda.node.classloading.LoaderTestFlow")
+        Assert.assertNotNull(initiatedFlows.find { it == expectedClass })
+    }
+}