Refactor BFTNotaryServicetest and MySQLNotaryServiceTest to allow running database integration tests. (#211)

* BFTNotaryServiceTests.kt - instantiate MockServices before each test, not at the class level - to allow database integration test clean database before each test.
* MySQLNotaryServiceTests.kt - use H2 datasource for the notary in database integration tests (and as it was before during standard integration tests)
* Revert Enterprise way of makeTestDataSourceProperties MockNode.kt.
* Minor attempt to refactor Enterprise only parts of reading database config and make it less error prone during OS->Enterprise merge.

node-api/src/main/kotlin/net/corda/nodeapi/internal/persistence/HibernateConfiguration.kt

@@ -6,12 +6,9 @@ import net.corda.core.utilities.contextLogger
 import net.corda.core.utilities.toHexString
 import org.hibernate.SessionFactory
 import org.hibernate.boot.MetadataSources
-import org.hibernate.boot.model.naming.Identifier
-import org.hibernate.boot.model.naming.PhysicalNamingStrategyStandardImpl
 import org.hibernate.boot.registry.BootstrapServiceRegistryBuilder
 import org.hibernate.cfg.Configuration
 import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider
-import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment
 import org.hibernate.service.UnknownUnwrapTypeException
 import org.hibernate.type.AbstractSingleColumnStandardBasicType
 import org.hibernate.type.descriptor.java.PrimitiveByteArrayTypeDescriptor

node/src/integration-test/kotlin/net/corda/node/services/BFTNotaryServiceTests.kt

@@ -50,13 +50,13 @@ class BFTNotaryServiceTests : IntegrationTest() {
         val databaseSchemas = IntegrationTestSchemas("node_0", "node_1", "node_2", "node_3", "node_4", "node_5",
                 "node_6", "node_7", "node_8", "node_9")
     }
-    private val mockNet = MockNetwork(emptyList())
+    private lateinit var mockNet: MockNetwork
     private lateinit var notary: Party
     private lateinit var node: StartedNode<MockNode>
 
     @Before
     fun before() {
-        node = mockNet.createNode()
+        mockNet = MockNetwork(emptyList())
     }
     @After
     fun stopNodes() {

node/src/integration-test/kotlin/net/corda/node/services/MySQLNotaryServiceTests.kt

@@ -23,11 +23,9 @@ import net.corda.testing.contracts.DummyContract
 import net.corda.testing.node.MockNetwork
 import net.corda.testing.node.MockNodeParameters
 import net.corda.testing.node.MockServices.Companion.makeTestDataSourceProperties
+import net.corda.testing.node.inMemoryH2DataSourceConfig
 import net.corda.testing.node.startFlow
-import org.junit.After
+import org.junit.*
-import org.junit.Before
-import org.junit.ClassRule
-import org.junit.Test
 import java.math.BigInteger
 import java.util.*
 import kotlin.test.assertEquals
@@ -36,9 +34,8 @@ import kotlin.test.assertFailsWith
 class MySQLNotaryServiceTests : IntegrationTest() {
     companion object {
         val notaryName = CordaX500Name("MySQL Notary Service", "Zurich", "CH")
-        @ClassRule
+        @ClassRule @JvmField
-        @JvmField
+        val databaseSchemas = IntegrationTestSchemas("node_0", "node_1", "node_2")
-        val databaseSchemas = IntegrationTestSchemas("node_0", DUMMY_NOTARY_NAME.toDatabaseSchemaName())
     }
 
     private lateinit var mockNet: MockNetwork
@@ -124,7 +121,7 @@ class MySQLNotaryServiceTests : IntegrationTest() {
     }
 
     private fun createNotaryNode(): MockNetwork.MockNode {
-        val dataStoreProperties = makeTestDataSourceProperties().apply {
+        val dataStoreProperties = makeTestDataSourceProperties(configSupplier = ::inMemoryH2DataSourceConfig).apply {
             setProperty("autoCommit", "false")
         }
         return mockNet.createUnstartedNode(

testing/node-driver/src/main/kotlin/net/corda/testing/node/MockNode.kt

@@ -401,7 +401,7 @@ open class MockNetwork(private val cordappPackages: List<String>,
         val config = mockNodeConfiguration().also {
             doReturn(baseDirectory(id).createDirectories()).whenever(it).baseDirectory
             doReturn(parameters.legalName ?: CordaX500Name("Mock Company $id", "London", "GB")).whenever(it).myLegalName
-            doReturn(makeTestDataSourceProperties("node_${id}_net_$networkId")).whenever(it).dataSourceProperties
+            doReturn(makeTestDataSourceProperties("node_$id","net_$networkId")).whenever(it).dataSourceProperties
             doReturn(makeTestDatabaseProperties("node_$id")).whenever(it).database
             parameters.configOverrides(it)
         }

testing/node-driver/src/main/kotlin/net/corda/testing/node/MockServices.kt

@@ -34,13 +34,13 @@ import net.corda.node.services.schema.HibernateObserver
 import net.corda.node.services.schema.NodeSchemaService
 import net.corda.node.services.transactions.InMemoryTransactionVerifierService
 import net.corda.node.services.vault.NodeVaultService
-import net.corda.node.internal.configureDatabase
 import net.corda.node.services.api.IdentityServiceInternal
 import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.nodeapi.internal.persistence.DatabaseConfig
 import net.corda.nodeapi.internal.persistence.HibernateConfiguration
 import net.corda.nodeapi.internal.persistence.TransactionIsolationLevel
 import net.corda.testing.*
+import net.corda.testing.database.DatabaseConstants
 import net.corda.testing.database.DatabaseConstants.DATA_SOURCE_CLASSNAME
 import net.corda.testing.database.DatabaseConstants.DATA_SOURCE_PASSWORD
 import net.corda.testing.database.DatabaseConstants.DATA_SOURCE_URL
@@ -75,51 +75,18 @@ open class MockServices private constructor(
         @JvmStatic
         val MOCK_VERSION_INFO = VersionInfo(1, "Mock release", "Mock revision", "Mock Vendor")
 
-        private fun readDatabaseConfig(nodeName: String? = null, postfix: String? = null): Config {
-
-            val parseOptions = ConfigParseOptions.defaults()
-
-            //read overrides from command line (passed by Gradle as system properties)
-            val dataSourceKeys = listOf(DATA_SOURCE_URL, DATA_SOURCE_CLASSNAME, DATA_SOURCE_USER, DATA_SOURCE_PASSWORD)
-            val dataSourceSystemProperties = Properties()
-            val allSystemProperties = System.getProperties().toList().map { it.first.toString() to it.second.toString() }.toMap()
-            dataSourceKeys.filter { allSystemProperties.containsKey(it) }.forEach { dataSourceSystemProperties.setProperty(it, allSystemProperties[it]) }
-            val systemConfigOverride = ConfigFactory.parseProperties(dataSourceSystemProperties, parseOptions)
-
-            //read from db vendor specific configuration file
-            val databaseConfig = ConfigFactory.parseResources(System.getProperty("databaseProvider") + ".conf", parseOptions.setAllowMissing(true))
-            val fixedOverride = ConfigFactory.parseString("baseDirectory = \"\"")
-
-            //implied property nodeOrganizationName to fill the potential placeholders in db schema/ db user properties
-            val standardizedNodeName = if (nodeName!= null) nodeName.replace(" ", "").replace("-", "_") else null
-            val nodeOrganizationNameConfig = if (standardizedNodeName != null) configOf("nodeOrganizationName" to standardizedNodeName) else ConfigFactory.empty()
-
-            //defaults to H2
-            //for H2 the same db instance runs for all integration tests, so adding additional variable postfix to use unique db user/schema each time
-            val h2InstanceName = if (postfix != null) standardizedNodeName + "_" + postfix else standardizedNodeName
-            val defaultProps = Properties()
-            defaultProps.setProperty(DATA_SOURCE_CLASSNAME, "org.h2.jdbcx.JdbcDataSource")
-            defaultProps.setProperty(DATA_SOURCE_URL, "jdbc:h2:mem:${h2InstanceName}_persistence;LOCK_TIMEOUT=10000;DB_CLOSE_ON_EXIT=FALSE")
-            defaultProps.setProperty(DATA_SOURCE_USER, "sa")
-            defaultProps.setProperty(DATA_SOURCE_PASSWORD, "")
-            val defaultConfig = ConfigFactory.parseProperties(defaultProps, parseOptions)
-
-            return systemConfigOverride.withFallback(databaseConfig)
-                    .withFallback(fixedOverride)
-                    .withFallback(nodeOrganizationNameConfig)
-                    .withFallback(defaultConfig)
-                    .resolve()
-        }
         /**
          * Make properties appropriate for creating a DataSource for unit tests.
          *
          * @param nodeName Reflects the "instance" of the in-memory database or database username/schema.  Defaults to a random string.
-         * @param nodeNameExtension Provides additional name extension for the "instance" of in-memory database to provide uniqnes when running unit tests against H2 db.
+         * @param nodeNameExtension Provides additional name extension for the "instance" of in-memory database to provide uniqueness when running unit tests against H2 db.
+         * @param configSupplier returns [Config] with dataSourceProperties entry.
          */
         // TODO: Can we use an X509 principal generator here?
         @JvmStatic
-        fun makeTestDataSourceProperties(nodeName: String = SecureHash.randomSHA256().toString(), nodeNameExtension: String? = null): Properties {
+        fun makeTestDataSourceProperties(nodeName: String = SecureHash.randomSHA256().toString(), nodeNameExtension: String? = null,
-            val config = readDatabaseConfig(nodeName, nodeNameExtension)
+                                         configSupplier: (String, String?) -> Config = ::databaseProviderDataSourceConfig): Properties {
+            val config = configSupplier(nodeName, nodeNameExtension)
             val props = Properties()
             props.setProperty("dataSourceClassName", config.getString("dataSourceProperties.dataSourceClassName"))
             props.setProperty("dataSource.url", config.getString("dataSourceProperties.dataSource.url"))
@@ -135,7 +102,7 @@ open class MockServices private constructor(
          */
         @JvmStatic
         fun makeTestDatabaseProperties(nodeName: String? = null): DatabaseConfig {
-            val config = readDatabaseConfig(nodeName)
+            val config = databaseProviderDataSourceConfig(nodeName)
             val transactionIsolationLevel = if (config.hasPath(TRANSACTION_ISOLATION_LEVEL)) TransactionIsolationLevel.valueOf(config.getString(TRANSACTION_ISOLATION_LEVEL))
                                                 else TransactionIsolationLevel.READ_COMMITTED
             val schema = if (config.hasPath(SCHEMA)) config.getString(SCHEMA) else ""
@@ -309,4 +276,57 @@ fun <T : SerializeAsToken> createMockCordaService(serviceHub: MockServices, serv
         }
     }
     return MockAppServiceHubImpl(serviceHub, serviceConstructor).serviceInstance
+}
+
+/**
+ * Reads database and dataSource configuration from a file denoted by 'databaseProvider' system property,
+ * overwitten by system properties and defaults to H2 in memory db.
+ * @param nodeName Reflects the "instance" of the database username/schema, the value will be used to replace ${nodeOrganizationName} placeholder
+ * if the placeholder is present in config.
+ * @param postfix Additional postix added to database "instance" name in case config defaults to H2 in memory database.
+ */
+fun databaseProviderDataSourceConfig(nodeName: String? = null, postfix: String? = null): Config {
+
+    val parseOptions = ConfigParseOptions.defaults()
+
+    //read overrides from command line (passed by Gradle as system properties)
+    val dataSourceKeys = listOf(DATA_SOURCE_URL, DATA_SOURCE_CLASSNAME, DATA_SOURCE_USER, DATA_SOURCE_PASSWORD)
+    val dataSourceSystemProperties = Properties()
+    val allSystemProperties = System.getProperties().toList().map { it.first.toString() to it.second.toString() }.toMap()
+    dataSourceKeys.filter { allSystemProperties.containsKey(it) }.forEach { dataSourceSystemProperties.setProperty(it, allSystemProperties[it]) }
+    val systemConfigOverride = ConfigFactory.parseProperties(dataSourceSystemProperties, parseOptions)
+
+    //read from db vendor specific configuration file
+    val databaseConfig = ConfigFactory.parseResources(System.getProperty("databaseProvider") + ".conf", parseOptions.setAllowMissing(true))
+    val fixedOverride = ConfigFactory.parseString("baseDirectory = \"\"")
+
+    //implied property nodeOrganizationName to fill the potential placeholders in db schema/ db user properties
+    val standardizedNodeName = nodeName?.replace(" ", "")?.replace("-", "_")
+    val nodeOrganizationNameConfig = if (standardizedNodeName != null) configOf("nodeOrganizationName" to standardizedNodeName) else ConfigFactory.empty()
+
+    //defaults to H2
+    //for H2 the same db instance runs for all integration tests, so adding additional variable postfix create a unique database each time
+    val defaultConfig = inMemoryH2DataSourceConfig(standardizedNodeName, postfix)
+
+    return systemConfigOverride.withFallback(databaseConfig)
+            .withFallback(fixedOverride)
+            .withFallback(nodeOrganizationNameConfig)
+            .withFallback(defaultConfig)
+            .resolve()
+}
+
+/**
+ * Creates data source configuration for in memory H2 as it would be specified in reference.conf 'datasource' snippet.
+ * @param nodeName Reflects the "instance" of the database username/schema
+ * @param postfix Additional postix added to database "instance" name to add uniqueness when running integration tests.
+ */
+fun inMemoryH2DataSourceConfig(nodeName: String? = null, postfix: String? = null) : Config {
+    val nodeName = nodeName ?: SecureHash.randomSHA256().toString()
+    val h2InstanceName = if (postfix != null) nodeName + "_" + postfix else nodeName
+
+    return ConfigFactory.parseMap(mapOf(
+            DatabaseConstants.DATA_SOURCE_CLASSNAME to "org.h2.jdbcx.JdbcDataSource",
+            DatabaseConstants.DATA_SOURCE_URL to "jdbc:h2:mem:${h2InstanceName}_persistence;LOCK_TIMEOUT=10000;DB_CLOSE_ON_EXIT=FALSE",
+            DatabaseConstants.DATA_SOURCE_USER to "sa",
+            DatabaseConstants.DATA_SOURCE_PASSWORD to ""))
 }