From 14e23430c0ea113dd5b32e97a4f1aaa0309d6c6f Mon Sep 17 00:00:00 2001 From: Adel El-Beik <48713346+adelel1@users.noreply.github.com> Date: Tue, 1 Sep 2020 10:30:49 +0100 Subject: [PATCH 1/2] CORDA-4003: Now support + in CorDapp filenames (#6673) * CORDA-4003: Now cope with file: prefix not being in class path element. * CORDA-4003: Switched to new URL type filter. * CORDA-4003: Switched to a URL comparison. In the string comparison the scheme was removed in latest version of classgraph. * CORDA-4003: Moved to latest version of classgraph that has support for + in filenames. * CORDA-4003: Switched to accept version of the deprecated classgraph methods. --- constants.properties | 2 +- .../corda/node/internal/cordapp/JarScanningCordappLoader.kt | 2 +- .../kotlin/net/corda/testing/node/internal/CustomCordapp.kt | 4 ++-- .../kotlin/net/corda/testing/node/internal/TestCordappImpl.kt | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/constants.properties b/constants.properties index 75f4e5f101..48f8ea263c 100644 --- a/constants.properties +++ b/constants.properties @@ -21,7 +21,7 @@ quasarVersion11=0.8.0_r3 jdkClassifier11=jdk11 proguardVersion=6.1.1 bouncycastleVersion=1.61 -classgraphVersion=4.8.78 +classgraphVersion=4.8.89 disruptorVersion=3.4.2 typesafeConfigVersion=1.3.4 jsr305Version=3.0.2 diff --git a/node/src/main/kotlin/net/corda/node/internal/cordapp/JarScanningCordappLoader.kt b/node/src/main/kotlin/net/corda/node/internal/cordapp/JarScanningCordappLoader.kt index 0975cabd68..87d859aee1 100644 --- a/node/src/main/kotlin/net/corda/node/internal/cordapp/JarScanningCordappLoader.kt +++ b/node/src/main/kotlin/net/corda/node/internal/cordapp/JarScanningCordappLoader.kt @@ -331,7 +331,7 @@ class JarScanningCordappLoader private constructor(private val cordappJarPaths: val cordappElement = cordappJarPath.url.toString() logger.info("Scanning CorDapp in $cordappElement") val scanResult = ClassGraph() - .filterClasspathElements { elt -> elt == cordappElement } + .filterClasspathElementsByURL { elt -> elt == cordappJarPath.url } .overrideClassLoaders(appClassLoader) .ignoreParentClassLoaders() .enableAllInfo() diff --git a/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/CustomCordapp.kt b/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/CustomCordapp.kt index 9d59807e95..ba28356ab8 100644 --- a/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/CustomCordapp.kt +++ b/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/CustomCordapp.kt @@ -56,11 +56,11 @@ data class CustomCordapp( internal fun packageAsJar(file: Path) { val classGraph = ClassGraph() if (packages.isNotEmpty()) { - classGraph.whitelistPaths(*packages.map { it.replace('.', '/') }.toTypedArray()) + classGraph.acceptPaths(*packages.map { it.replace('.', '/') }.toTypedArray()) } if (classes.isNotEmpty()) { classes.forEach { classGraph.addClassLoader(it.classLoader) } - classGraph.whitelistClasses(*classes.map { it.name }.toTypedArray()) + classGraph.acceptClasses(*classes.map { it.name }.toTypedArray()) } classGraph.enableClassInfo().pooledScan().use { scanResult -> diff --git a/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/TestCordappImpl.kt b/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/TestCordappImpl.kt index 40cc21ec93..68b37b5247 100644 --- a/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/TestCordappImpl.kt +++ b/testing/node-driver/src/main/kotlin/net/corda/testing/node/internal/TestCordappImpl.kt @@ -56,7 +56,7 @@ data class TestCordappImpl(val scanPackage: String, override val config: Map { return packageToRootPaths.computeIfAbsent(scanPackage) { - val classGraph = ClassGraph().whitelistPaths(scanPackage.replace('.', '/')) + val classGraph = ClassGraph().acceptPaths(scanPackage.replace('.', '/')) classGraph.pooledScan().use { scanResult -> scanResult.allResources .asSequence() From 6945426ef1605830a498159b3ce7f8a9ea4c1b3b Mon Sep 17 00:00:00 2001 From: Ross Nicoll Date: Tue, 1 Sep 2020 16:04:12 +0100 Subject: [PATCH 2/2] NOTICK Robustness improvements for DB race condition test (#6556) (#6677) Switch away from log monitoring to verify that DB race conditions are reported correctly, in an attempt to resolve a test instability issue. --- .../persistence/DBTransactionStorage.kt | 22 +++++-- .../persistence/DBTransactionStorageTests.kt | 58 ++++--------------- 2 files changed, 29 insertions(+), 51 deletions(-) diff --git a/node/src/main/kotlin/net/corda/node/services/persistence/DBTransactionStorage.kt b/node/src/main/kotlin/net/corda/node/services/persistence/DBTransactionStorage.kt index 7084768069..165e2c1f75 100644 --- a/node/src/main/kotlin/net/corda/node/services/persistence/DBTransactionStorage.kt +++ b/node/src/main/kotlin/net/corda/node/services/persistence/DBTransactionStorage.kt @@ -95,7 +95,9 @@ class DBTransactionStorage(private val database: CordaPersistence, cacheFactory: } } - private companion object { + internal companion object { + const val TRANSACTION_ALREADY_IN_PROGRESS_WARNING = "trackTransaction is called with an already existing, open DB transaction. As a result, there might be transactions missing from the returned data feed, because of race conditions." + // Rough estimate for the average of a public key and the transaction metadata - hard to get exact figures here, // as public keys can vary in size a lot, and if someone else is holding a reference to the key, it won't add // to the memory pressure at all here. @@ -111,7 +113,7 @@ class DBTransactionStorage(private val database: CordaPersistence, cacheFactory: } } - fun createTransactionsMap(cacheFactory: NamedCacheFactory, clock: CordaClock) + private fun createTransactionsMap(cacheFactory: NamedCacheFactory, clock: CordaClock) : AppendOnlyPersistentMapBase { return WeightBasedAppendOnlyPersistentMap( cacheFactory = cacheFactory, @@ -221,12 +223,22 @@ class DBTransactionStorage(private val database: CordaPersistence, cacheFactory: } override fun trackTransaction(id: SecureHash): CordaFuture { + val (transaction, warning) = trackTransactionInternal(id) + warning?.also { log.warn(it) } + return transaction + } - if (contextTransactionOrNull != null) { - log.warn("trackTransaction is called with an already existing, open DB transaction. As a result, there might be transactions missing from the returned data feed, because of race conditions.") + /** + * @return a pair of the signed transaction, and a string containing any warning. + */ + internal fun trackTransactionInternal(id: SecureHash): Pair, String?> { + val warning: String? = if (contextTransactionOrNull != null) { + TRANSACTION_ALREADY_IN_PROGRESS_WARNING + } else { + null } - return trackTransactionWithNoWarning(id) + return Pair(trackTransactionWithNoWarning(id), warning) } override fun trackTransactionWithNoWarning(id: SecureHash): CordaFuture { diff --git a/node/src/test/kotlin/net/corda/node/services/persistence/DBTransactionStorageTests.kt b/node/src/test/kotlin/net/corda/node/services/persistence/DBTransactionStorageTests.kt index 39155a335a..51b400c321 100644 --- a/node/src/test/kotlin/net/corda/node/services/persistence/DBTransactionStorageTests.kt +++ b/node/src/test/kotlin/net/corda/node/services/persistence/DBTransactionStorageTests.kt @@ -9,22 +9,22 @@ import net.corda.core.crypto.SignatureMetadata import net.corda.core.crypto.TransactionSignature import net.corda.core.toFuture import net.corda.core.transactions.SignedTransaction -import net.corda.node.services.transactions.PersistentUniquenessProvider import net.corda.node.CordaClock import net.corda.node.MutableClock import net.corda.node.SimpleClock +import net.corda.node.services.transactions.PersistentUniquenessProvider import net.corda.nodeapi.internal.persistence.CordaPersistence import net.corda.nodeapi.internal.persistence.DatabaseConfig -import net.corda.testing.core.* +import net.corda.testing.core.ALICE_NAME +import net.corda.testing.core.DUMMY_NOTARY_NAME +import net.corda.testing.core.SerializationEnvironmentRule +import net.corda.testing.core.TestIdentity +import net.corda.testing.core.dummyCommand import net.corda.testing.internal.LogHelper import net.corda.testing.internal.TestingNamedCacheFactory import net.corda.testing.internal.configureDatabase import net.corda.testing.internal.createWireTransaction import net.corda.testing.node.MockServices.Companion.makeTestDataSourceProperties -import org.apache.logging.log4j.LogManager -import org.apache.logging.log4j.core.Appender -import org.apache.logging.log4j.core.LoggerContext -import org.apache.logging.log4j.core.appender.WriterAppender import org.assertj.core.api.Assertions.assertThat import org.junit.After import org.junit.Assert @@ -32,10 +32,9 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import rx.plugins.RxJavaHooks -import java.io.StringWriter -import java.util.concurrent.Semaphore import java.time.Clock import java.time.Instant +import java.util.concurrent.Semaphore import java.util.concurrent.TimeUnit import kotlin.concurrent.thread import kotlin.test.assertEquals @@ -381,47 +380,14 @@ class DBTransactionStorageTests { val signedTransaction = newTransaction() // Act - val logMessages = collectLogsFrom { - database.transaction { - val result = transactionStorage.trackTransaction(signedTransaction.id) - result.cancel(false) - } + val warning = database.transaction { + val (result, warning) = transactionStorage.trackTransactionInternal(signedTransaction.id) + result.cancel(false) + warning } // Assert - assertThat(logMessages).contains("trackTransaction is called with an already existing, open DB transaction. As a result, there might be transactions missing from the returned data feed, because of race conditions.") - } - - private fun collectLogsFrom(statement: () -> Unit): String { - // Create test appender - val stringWriter = StringWriter() - val appenderName = this::collectLogsFrom.name - val appender: Appender = WriterAppender.createAppender( - null, - null, - stringWriter, - appenderName, - false, - true - ) - appender.start() - - // Add test appender - val context = LogManager.getContext(false) as LoggerContext - val configuration = context.configuration - configuration.addAppender(appender) - configuration.loggers.values.forEach { it.addAppender(appender, null, null) } - - try { - statement() - } finally { - // Remove test appender - configuration.loggers.values.forEach { it.removeAppender(appenderName) } - configuration.appenders.remove(appenderName) - appender.stop() - } - - return stringWriter.toString() + assertThat(warning).isEqualTo(DBTransactionStorage.TRANSACTION_ALREADY_IN_PROGRESS_WARNING) } private fun newTransactionStorage(cacheSizeBytesOverride: Long? = null, clock: CordaClock = SimpleClock(Clock.systemUTC())) {