CORDA-1385: Ignore duplicate packages and sub-packages in driver extraCordappPackagesToScan (#3066)

Otherwise duplicate test CorDapps are loaded into the node
This commit is contained in:
Shams Asari 2018-05-03 12:28:36 +01:00 committed by GitHub
parent a6b7257491
commit 9ffb43f3f7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 68 additions and 51 deletions

View File

@ -37,13 +37,13 @@ class CordappScanningDriverTest {
@StartableByRPC @StartableByRPC
@InitiatingFlow @InitiatingFlow
class ReceiveFlow(val otherParty: Party) : FlowLogic<String>() { class ReceiveFlow(private val otherParty: Party) : FlowLogic<String>() {
@Suspendable @Suspendable
override fun call(): String = initiateFlow(otherParty).receive<String>().unwrap { it } override fun call(): String = initiateFlow(otherParty).receive<String>().unwrap { it }
} }
@InitiatedBy(ReceiveFlow::class) @InitiatedBy(ReceiveFlow::class)
open class SendClassFlow(val otherPartySession: FlowSession) : FlowLogic<Unit>() { open class SendClassFlow(private val otherPartySession: FlowSession) : FlowLogic<Unit>() {
@Suspendable @Suspendable
override fun call() = otherPartySession.send(javaClass.name) override fun call() = otherPartySession.send(javaClass.name)
} }

View File

@ -86,8 +86,10 @@ open class Node(configuration: NodeConfiguration,
} }
private val sameVmNodeCounter = AtomicInteger() private val sameVmNodeCounter = AtomicInteger()
val scanPackagesSystemProperty = "net.corda.node.cordapp.scan.packages"
val scanPackagesSeparator = "," const val scanPackagesSystemProperty = "net.corda.node.cordapp.scan.packages"
const val scanPackagesSeparator = ","
private fun makeCordappLoader(configuration: NodeConfiguration): CordappLoader { private fun makeCordappLoader(configuration: NodeConfiguration): CordappLoader {
return System.getProperty(scanPackagesSystemProperty)?.let { scanPackages -> return System.getProperty(scanPackagesSystemProperty)?.let { scanPackages ->
CordappLoader.createDefaultWithTestPackages(configuration, scanPackages.split(scanPackagesSeparator)) CordappLoader.createDefaultWithTestPackages(configuration, scanPackages.split(scanPackagesSeparator))

View File

@ -1,10 +1,8 @@
package net.corda.node.internal.cordapp package net.corda.node.internal.cordapp
import com.github.benmanes.caffeine.cache.Caffeine
import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner import io.github.lukehutch.fastclasspathscanner.FastClasspathScanner
import io.github.lukehutch.fastclasspathscanner.scanner.ScanResult import io.github.lukehutch.fastclasspathscanner.scanner.ScanResult
import net.corda.core.contracts.Contract
import net.corda.core.contracts.UpgradedContract
import net.corda.core.contracts.UpgradedContractWithLegacyConstraint
import net.corda.core.cordapp.Cordapp import net.corda.core.cordapp.Cordapp
import net.corda.core.flows.* import net.corda.core.flows.*
import net.corda.core.internal.* import net.corda.core.internal.*
@ -22,7 +20,6 @@ import net.corda.nodeapi.internal.serialization.DefaultWhitelist
import org.apache.commons.collections4.map.LRUMap import org.apache.commons.collections4.map.LRUMap
import java.lang.reflect.Modifier import java.lang.reflect.Modifier
import java.net.JarURLConnection import java.net.JarURLConnection
import java.net.URI
import java.net.URL import java.net.URL
import java.net.URLClassLoader import java.net.URLClassLoader
import java.nio.file.Path import java.nio.file.Path
@ -30,6 +27,7 @@ import java.nio.file.Paths
import java.nio.file.attribute.FileTime import java.nio.file.attribute.FileTime
import java.time.Instant import java.time.Instant
import java.util.* import java.util.*
import java.util.concurrent.ConcurrentHashMap
import java.util.jar.JarOutputStream import java.util.jar.JarOutputStream
import java.util.zip.ZipEntry import java.util.zip.ZipEntry
import kotlin.reflect.KClass import kotlin.reflect.KClass
@ -67,10 +65,21 @@ class CordappLoader private constructor(private val cordappJarPaths: List<Restri
* @param baseDir The directory that this node is running in. Will use this to resolve the cordapps directory * @param baseDir The directory that this node is running in. Will use this to resolve the cordapps directory
* for classpath scanning. * for classpath scanning.
*/ */
fun createDefault(baseDir: Path) = CordappLoader(getCordappsInDirectory(getCordappsPath(baseDir))) fun createDefault(baseDir: Path) = CordappLoader(getNodeCordappURLs(baseDir))
// Cache for CordappLoaders to avoid costly classpath scanning // Cache for CordappLoaders to avoid costly classpath scanning
private val cordappLoadersCache = LRUMap<List<*>, CordappLoader>(1000) private val cordappLoadersCache = Caffeine.newBuilder().softValues().build<List<RestrictedURL>, CordappLoader>()
private val generatedCordapps = ConcurrentHashMap<URL, Path>()
private fun simplifyScanPackages(scanPackages: List<String>): List<String> {
return scanPackages.sorted().fold(emptyList()) { listSoFar, packageName ->
when {
listSoFar.isEmpty() -> listOf(packageName)
packageName.startsWith(listSoFar.last()) -> listSoFar // Squash ["com.foo", "com.foo.bar"] into just ["com.foo"]
else -> listSoFar + packageName
}
}
}
/** /**
* Create a dev mode CordappLoader for test environments that creates and loads cordapps from the classpath * Create a dev mode CordappLoader for test environments that creates and loads cordapps from the classpath
@ -83,8 +92,8 @@ class CordappLoader private constructor(private val cordappJarPaths: List<Restri
if (!configuration.devMode) { if (!configuration.devMode) {
logger.warn("Package scanning should only occur in dev mode!") logger.warn("Package scanning should only occur in dev mode!")
} }
val paths = getCordappsInDirectory(getCordappsPath(configuration.baseDirectory)) + testPackages.flatMap(this::createScanPackage) val urls = getNodeCordappURLs(configuration.baseDirectory) + simplifyScanPackages(testPackages).flatMap(this::getPackageURLs)
return cordappLoadersCache.computeIfAbsent(paths, { CordappLoader(paths) }) return cordappLoadersCache.asMap().computeIfAbsent(urls, ::CordappLoader)
} }
/** /**
@ -96,7 +105,8 @@ class CordappLoader private constructor(private val cordappJarPaths: List<Restri
*/ */
@VisibleForTesting @VisibleForTesting
fun createWithTestPackages(testPackages: List<String>): CordappLoader { fun createWithTestPackages(testPackages: List<String>): CordappLoader {
return cordappLoadersCache.computeIfAbsent(testPackages, { CordappLoader(testPackages.flatMap(this::createScanPackage)) }) val urls = simplifyScanPackages(testPackages).flatMap(this::getPackageURLs)
return cordappLoadersCache.asMap().computeIfAbsent(urls, ::CordappLoader)
} }
/** /**
@ -107,34 +117,33 @@ class CordappLoader private constructor(private val cordappJarPaths: List<Restri
@VisibleForTesting @VisibleForTesting
fun createDevMode(scanJars: List<URL>) = CordappLoader(scanJars.map { RestrictedURL(it, null) }) fun createDevMode(scanJars: List<URL>) = CordappLoader(scanJars.map { RestrictedURL(it, null) })
private fun getCordappsPath(baseDir: Path): Path = baseDir / CORDAPPS_DIR_NAME private fun getPackageURLs(scanPackage: String): List<RestrictedURL> {
private fun createScanPackage(scanPackage: String): List<RestrictedURL> {
val resource = scanPackage.replace('.', '/') val resource = scanPackage.replace('.', '/')
return this::class.java.classLoader.getResources(resource) return this::class.java.classLoader.getResources(resource)
.asSequence() .asSequence()
.map { path -> .map { url ->
if (path.protocol == "jar") { if (url.protocol == "jar") {
// When running tests from gradle this may be a corda module jar, so restrict to scanPackage: // When running tests from gradle this may be a corda module jar, so restrict to scanPackage:
RestrictedURL((path.openConnection() as JarURLConnection).jarFileURL, scanPackage) RestrictedURL((url.openConnection() as JarURLConnection).jarFileURL, scanPackage)
} else { } else {
// No need to restrict as createDevCordappJar has already done that: // No need to restrict as createDevCordappJar has already done that:
RestrictedURL(createDevCordappJar(scanPackage, path, resource).toURL(), null) RestrictedURL(createDevCordappJar(scanPackage, url, resource).toUri().toURL(), null)
} }
} }
.toList() .toList()
} }
/** Takes a package of classes and creates a JAR from them - only use in tests. */ /** Takes a package of classes and creates a JAR from them - only use in tests. */
private fun createDevCordappJar(scanPackage: String, url: URL, jarPackageName: String): URI { private fun createDevCordappJar(scanPackage: String, url: URL, resource: String): Path {
return generatedCordapps.computeIfAbsent(url) { return generatedCordapps.computeIfAbsent(url) {
// TODO Using the driver in out-of-process mode causes each node to have their own copy of the same dev CorDapps
val cordappDir = (Paths.get("build") / "tmp" / "generated-test-cordapps").createDirectories() val cordappDir = (Paths.get("build") / "tmp" / "generated-test-cordapps").createDirectories()
val cordappJAR = cordappDir / "$scanPackage-${UUID.randomUUID()}.jar" val cordappJar = cordappDir / "$scanPackage-${UUID.randomUUID()}.jar"
logger.info("Generating a test-only cordapp of classes discovered in $scanPackage at $cordappJAR") logger.info("Generating a test-only CorDapp of classes discovered for package $scanPackage in $url: $cordappJar")
JarOutputStream(cordappJAR.outputStream()).use { jos -> JarOutputStream(cordappJar.outputStream()).use { jos ->
val scanDir = url.toPath() val scanDir = url.toPath()
scanDir.walk { it.forEach { scanDir.walk { it.forEach {
val entryPath = "$jarPackageName/${scanDir.relativize(it).toString().replace('\\', '/')}" val entryPath = "$resource/${scanDir.relativize(it).toString().replace('\\', '/')}"
val time = FileTime.from(Instant.EPOCH) val time = FileTime.from(Instant.EPOCH)
val entry = ZipEntry(entryPath).setCreationTime(time).setLastAccessTime(time).setLastModifiedTime(time) val entry = ZipEntry(entryPath).setCreationTime(time).setLastAccessTime(time).setLastModifiedTime(time)
jos.putNextEntry(entry) jos.putNextEntry(entry)
@ -144,22 +153,21 @@ class CordappLoader private constructor(private val cordappJarPaths: List<Restri
jos.closeEntry() jos.closeEntry()
} } } }
} }
cordappJAR.toUri() cordappJar
} }
} }
private fun getCordappsInDirectory(cordappsDir: Path): List<RestrictedURL> { private fun getNodeCordappURLs(baseDir: Path): List<RestrictedURL> {
val cordappsDir = baseDir / CORDAPPS_DIR_NAME
return if (!cordappsDir.exists()) { return if (!cordappsDir.exists()) {
emptyList() emptyList()
} else { } else {
cordappsDir.list { cordappsDir.list {
it.filter { it.isRegularFile() && it.toString().endsWith(".jar") }.map { RestrictedURL(it.toUri().toURL(), null) }.toList() it.filter { it.toString().endsWith(".jar") }.map { RestrictedURL(it.toUri().toURL(), null) }.toList()
} }
} }
} }
private val generatedCordapps = mutableMapOf<URL, URI>()
/** A list of the core RPC flows present in Corda */ /** A list of the core RPC flows present in Corda */
private val coreRPCFlows = listOf( private val coreRPCFlows = listOf(
ContractUpgradeFlow.Initiate::class.java, ContractUpgradeFlow.Initiate::class.java,
@ -253,7 +261,7 @@ class CordappLoader private constructor(private val cordappJarPaths: List<Restri
private val cachedScanResult = LRUMap<RestrictedURL, RestrictedScanResult>(1000) private val cachedScanResult = LRUMap<RestrictedURL, RestrictedScanResult>(1000)
private fun scanCordapp(cordappJarPath: RestrictedURL): RestrictedScanResult { private fun scanCordapp(cordappJarPath: RestrictedURL): RestrictedScanResult {
logger.info("Scanning CorDapp in $cordappJarPath") logger.info("Scanning CorDapp in ${cordappJarPath.url}")
return cachedScanResult.computeIfAbsent(cordappJarPath, { return cachedScanResult.computeIfAbsent(cordappJarPath, {
RestrictedScanResult(FastClasspathScanner().addClassLoader(appClassLoader).overrideClasspath(cordappJarPath.url).scan(), cordappJarPath.qualifiedNamePrefix) RestrictedScanResult(FastClasspathScanner().addClassLoader(appClassLoader).overrideClasspath(cordappJarPath.url).scan(), cordappJarPath.qualifiedNamePrefix)
}) })
@ -283,10 +291,9 @@ class CordappLoader private constructor(private val cordappJarPaths: List<Restri
} }
} }
/** @param rootPackageName only this package and subpackages may be extracted from [url], or null to allow all packages. */ /** @property rootPackageName only this package and subpackages may be extracted from [url], or null to allow all packages. */
private class RestrictedURL(val url: URL, rootPackageName: String?) { private data class RestrictedURL(val url: URL, val rootPackageName: String?) {
val qualifiedNamePrefix = rootPackageName?.let { it + '.' } ?: "" val qualifiedNamePrefix: String get() = rootPackageName?.let { it + '.' } ?: ""
override fun toString() = url.toString()
} }
private inner class RestrictedScanResult(private val scanResult: ScanResult, private val qualifiedNamePrefix: String) { private inner class RestrictedScanResult(private val scanResult: ScanResult, private val qualifiedNamePrefix: String) {

View File

@ -9,45 +9,39 @@ import java.nio.file.Paths
@InitiatingFlow @InitiatingFlow
class DummyFlow : FlowLogic<Unit>() { class DummyFlow : FlowLogic<Unit>() {
@Suspendable @Suspendable
override fun call() { override fun call() = Unit
}
} }
@InitiatedBy(DummyFlow::class) @InitiatedBy(DummyFlow::class)
class LoaderTestFlow(unusedSession: FlowSession) : FlowLogic<Unit>() { class LoaderTestFlow(@Suppress("UNUSED_PARAMETER") unusedSession: FlowSession) : FlowLogic<Unit>() {
@Suspendable @Suspendable
override fun call() { override fun call() = Unit
}
} }
@SchedulableFlow @SchedulableFlow
class DummySchedulableFlow : FlowLogic<Unit>() { class DummySchedulableFlow : FlowLogic<Unit>() {
@Suspendable @Suspendable
override fun call() { override fun call() = Unit
}
} }
@StartableByRPC @StartableByRPC
class DummyRPCFlow : FlowLogic<Unit>() { class DummyRPCFlow : FlowLogic<Unit>() {
@Suspendable @Suspendable
override fun call() { override fun call() = Unit
}
} }
class CordappLoaderTest { class CordappLoaderTest {
private companion object { private companion object {
val testScanPackages = listOf("net.corda.node.internal.cordapp") const val testScanPackage = "net.corda.node.internal.cordapp"
val isolatedContractId = "net.corda.finance.contracts.isolated.AnotherDummyContract" const val isolatedContractId = "net.corda.finance.contracts.isolated.AnotherDummyContract"
val isolatedFlowName = "net.corda.finance.contracts.isolated.IsolatedDummyFlow\$Initiator" const val isolatedFlowName = "net.corda.finance.contracts.isolated.IsolatedDummyFlow\$Initiator"
} }
@Test @Test
fun `test that classes that aren't in cordapps aren't loaded`() { 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 // 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(".")) val loader = CordappLoader.createDefault(Paths.get("."))
assertThat(loader.cordapps) assertThat(loader.cordapps).containsOnly(CordappLoader.coreCordapp)
.hasSize(1)
.contains(CordappLoader.coreCordapp)
} }
@Test @Test
@ -71,7 +65,7 @@ class CordappLoaderTest {
@Test @Test
fun `flows are loaded by loader`() { fun `flows are loaded by loader`() {
val loader = CordappLoader.createWithTestPackages(testScanPackages) val loader = CordappLoader.createWithTestPackages(listOf(testScanPackage))
val actual = loader.cordapps.toTypedArray() val actual = loader.cordapps.toTypedArray()
// One core cordapp, one cordapp from this source tree, and two others due to identically named locations // One core cordapp, one cordapp from this source tree, and two others due to identically named locations
@ -85,6 +79,20 @@ class CordappLoaderTest {
assertThat(actualCordapp.schedulableFlows).first().hasSameClassAs(DummySchedulableFlow::class.java) assertThat(actualCordapp.schedulableFlows).first().hasSameClassAs(DummySchedulableFlow::class.java)
} }
@Test
fun `duplicate packages are ignored`() {
val loader = CordappLoader.createWithTestPackages(listOf(testScanPackage, testScanPackage))
val cordapps = loader.cordapps.filter { LoaderTestFlow::class.java in it.initiatedFlows }
assertThat(cordapps).hasSize(1)
}
@Test
fun `sub-packages are ignored`() {
val loader = CordappLoader.createWithTestPackages(listOf("net.corda", testScanPackage))
val cordapps = loader.cordapps.filter { LoaderTestFlow::class.java in it.initiatedFlows }
assertThat(cordapps).hasSize(1)
}
// This test exists because the appClassLoader is used by serialisation and we need to ensure it is the classloader // This test exists because the appClassLoader is used by serialisation and we need to ensure it is the classloader
// being used internally. Later iterations will use a classloader per cordapp and this test can be retired. // being used internally. Later iterations will use a classloader per cordapp and this test can be retired.
@Test @Test