CORDA-3302: Make liqubase migration failures warn on H2 unless explicity asked for (#5556)

* Make liqubase migration failures warn on H2 unless explicity asked for

* delete buildSrc block configuring multiple plugins (#5565)

* Add test for new option

* CORDA-3302: Make liqubase migration failures warn on H2 unless explicity asked for -  pass all node's core MappedSchemas to the test

* Make function name more meaningful

* Fix compilation error

detekt-baseline.xml

@@ -3258,9 +3258,9 @@
     <ID>MaxLineLength:SchemaMigration.kt$MissingMigrationException.Companion$fun errorMessageFor(mappedSchema: MappedSchema): String</ID>
     <ID>MaxLineLength:SchemaMigration.kt$OutstandingDatabaseChangesException : DatabaseMigrationException</ID>
     <ID>MaxLineLength:SchemaMigration.kt$SchemaMigration$ private fun migrateOlderDatabaseToUseLiquibase(existingCheckpoints: Boolean): Boolean</ID>
+    <ID>MaxLineLength:SchemaMigration.kt$SchemaMigration$( val schemas: Set&lt;MappedSchema&gt;, val dataSource: DataSource, private val databaseConfig: DatabaseConfig, cordappLoader: CordappLoader? = null, private val currentDirectory: Path?, // This parameter is used by the vault state migration to establish what the node's legal identity is when setting up // its copy of the identity service. It is passed through using a system property. When multiple identity support is added, this will need // reworking so that multiple identities can be passed to the migration. private val ourName: CordaX500Name, // This parameter forces an error to be thrown if there are missing migrations. When using H2, Hibernate will automatically create schemas where they are // missing, so no need to throw unless you're specifically testing whether all the migrations are present. private val forceThrowOnMissingMigration: Boolean = false)</ID>
     <ID>MaxLineLength:SchemaMigration.kt$SchemaMigration$(mappedSchema::class.qualifiedName == "net.corda.finance.schemas.CashSchemaV1" || mappedSchema::class.qualifiedName == "net.corda.finance.schemas.CommercialPaperSchemaV1") &amp;&amp; mappedSchema.migrationResource == null -&gt; null</ID>
     <ID>MaxLineLength:SchemaMigration.kt$SchemaMigration$(run &amp;&amp; !check) &amp;&amp; (unRunChanges.isNotEmpty() &amp;&amp; existingCheckpoints!!) -&gt; throw CheckpointsException()</ID>
-    <ID>MaxLineLength:SchemaMigration.kt$SchemaMigration$// Collect all changelog file referenced in the included schemas. // For backward compatibility reasons, when failOnMigrationMissing=false, we don't manage CorDapps via Liquibase but use the hibernate hbm2ddl=update. val changelogList = schemas.mapNotNull { mappedSchema -&gt; val resource = getMigrationResource(mappedSchema, classLoader) when { resource != null -&gt; resource // Corda OS FinanceApp in v3 has no Liquibase script, so no error is raised (mappedSchema::class.qualifiedName == "net.corda.finance.schemas.CashSchemaV1" || mappedSchema::class.qualifiedName == "net.corda.finance.schemas.CommercialPaperSchemaV1") &amp;&amp; mappedSchema.migrationResource == null -&gt; null else -&gt; throw MissingMigrationException(mappedSchema) } } val path = currentDirectory?.toString() if (path != null) { System.setProperty(NODE_BASE_DIR_KEY, path) // base dir for any custom change set which may need to load a file (currently AttachmentVersionNumberMigration) } System.setProperty(NODE_X500_NAME, ourName.toString()) val customResourceAccessor = CustomResourceAccessor(dynamicInclude, changelogList, classLoader) checkResourcesInClassPath(changelogList) // current version of Liquibase appears to be non-threadsafe // this is apparent when multiple in-process nodes are all running migrations simultaneously mutex.withLock { val liquibase = Liquibase(dynamicInclude, customResourceAccessor, getLiquibaseDatabase(JdbcConnection(connection))) val unRunChanges = liquibase.listUnrunChangeSets(Contexts(), LabelExpression()) when { (run &amp;&amp; !check) &amp;&amp; (unRunChanges.isNotEmpty() &amp;&amp; existingCheckpoints!!) -&gt; throw CheckpointsException() // Do not allow database migration when there are checkpoints run &amp;&amp; !check -&gt; liquibase.update(Contexts()) check &amp;&amp; !run &amp;&amp; unRunChanges.isNotEmpty() -&gt; throw OutstandingDatabaseChangesException(unRunChanges.size) check &amp;&amp; !run -&gt; { } // Do nothing will be interpreted as "check succeeded" else -&gt; throw IllegalStateException("Invalid usage.") } }</ID>
     <ID>MaxLineLength:SchemaMigration.kt$SchemaMigration$System.setProperty(NODE_BASE_DIR_KEY, path)</ID>
     <ID>MaxLineLength:SchemaMigration.kt$SchemaMigration$it.execute("SELECT COUNT(*) FROM DATABASECHANGELOG WHERE FILENAME IN ('migration/cash.changelog-init.xml','migration/commercial-paper.changelog-init.xml')")</ID>
     <ID>MaxLineLength:SchemaMigration.kt$SchemaMigration$private</ID>

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

@@ -22,17 +22,19 @@ import java.util.concurrent.locks.ReentrantLock
 import kotlin.concurrent.withLock
 
 // Migrate the database to the current version, using liquibase.
-//
-// A note on the ourName parameter: This is used by the vault state migration to establish what the node's legal identity is when setting up
-// its copy of the identity service. It is passed through using a system property. When multiple identity support is added, this will need
-// reworking so that multiple identities can be passed to the migration.
 class SchemaMigration(
         val schemas: Set<MappedSchema>,
         val dataSource: DataSource,
         private val databaseConfig: DatabaseConfig,
         cordappLoader: CordappLoader? = null,
         private val currentDirectory: Path?,
-        private val ourName: CordaX500Name) {
+        // This parameter is used by the vault state migration to establish what the node's legal identity is when setting up
+        // its copy of the identity service. It is passed through using a system property. When multiple identity support is added, this will need
+        // reworking so that multiple identities can be passed to the migration.
+        private val ourName: CordaX500Name,
+        // This parameter forces an error to be thrown if there are missing migrations. When using H2, Hibernate will automatically create schemas where they are
+        // missing, so no need to throw unless you're specifically testing whether all the migrations are present.
+        private val forceThrowOnMissingMigration: Boolean = false) {
 
     companion object {
         private val logger = contextLogger()
@@ -89,6 +91,14 @@ class SchemaMigration(
         }
     }
 
+    private fun logOrThrowMigrationError(mappedSchema: MappedSchema): String? =
+            if (forceThrowOnMissingMigration) {
+                throw MissingMigrationException(mappedSchema)
+            } else {
+                logger.warn(MissingMigrationException.errorMessageFor(mappedSchema))
+                null
+            }
+
     private fun doRunMigration(run: Boolean, check: Boolean, existingCheckpoints: Boolean? = null) {
 
         // Virtual file name of the changelog that includes all schemas.
@@ -96,15 +106,14 @@ class SchemaMigration(
 
         dataSource.connection.use { connection ->
 
-            // Collect all changelog file referenced in the included schemas.
+            // Collect all changelog files referenced in the included schemas.
-            // For backward compatibility reasons, when failOnMigrationMissing=false, we don't manage CorDapps via Liquibase but use the hibernate hbm2ddl=update.
             val changelogList = schemas.mapNotNull { mappedSchema ->
                 val resource = getMigrationResource(mappedSchema, classLoader)
                 when {
                     resource != null -> resource
                     // Corda OS FinanceApp in v3 has no Liquibase script, so no error is raised
                     (mappedSchema::class.qualifiedName == "net.corda.finance.schemas.CashSchemaV1" || mappedSchema::class.qualifiedName == "net.corda.finance.schemas.CommercialPaperSchemaV1") && mappedSchema.migrationResource == null -> null
-                    else -> throw MissingMigrationException(mappedSchema)
+                    else -> logOrThrowMigrationError(mappedSchema)
                 }
             }
 
@@ -163,7 +172,7 @@ class SchemaMigration(
             val isFinanceAppWithLiquibaseNotMigrated = isFinanceAppWithLiquibase // If Finance App is pre v4.0 then no need to migrate it so no need to check.
                     && existingDatabase
                     && (!hasLiquibase // Migrate as other tables.
-                         || (hasLiquibase && it.createStatement().use { noLiquibaseEntryLogForFinanceApp(it) })) // If Liquibase is already in the database check if Finance App schema log is missing.
+                    || (hasLiquibase && it.createStatement().use { noLiquibaseEntryLogForFinanceApp(it) })) // If Liquibase is already in the database check if Finance App schema log is missing.
 
             Pair(existingDatabase && !hasLiquibase, isFinanceAppWithLiquibaseNotMigrated)
         }

node-api/src/test/kotlin/net/corda/nodeapi/internal/SchemaMigrationTest.kt

@@ -0,0 +1,74 @@
+package net.corda.nodeapi.internal
+
+import net.corda.core.schemas.MappedSchema
+import net.corda.core.schemas.PersistentState
+import net.corda.node.internal.DataSourceFactory
+import net.corda.node.services.persistence.DBCheckpointStorage
+import net.corda.node.services.schema.NodeSchemaService
+import net.corda.nodeapi.internal.persistence.DatabaseConfig
+import net.corda.nodeapi.internal.persistence.MissingMigrationException
+import net.corda.nodeapi.internal.persistence.SchemaMigration
+import net.corda.testing.core.ALICE_NAME
+import net.corda.testing.core.TestIdentity
+import net.corda.testing.node.MockServices
+import org.assertj.core.api.Assertions.assertThatThrownBy
+import org.junit.Before
+import org.junit.Test
+import org.junit.jupiter.api.assertDoesNotThrow
+import java.util.*
+import javax.persistence.Column
+import javax.persistence.Entity
+import javax.sql.DataSource
+
+class SchemaMigrationTest {
+    object TestSchemaFamily
+
+    object GoodSchema : MappedSchema(schemaFamily = TestSchemaFamily.javaClass, version = 1, mappedTypes = listOf(State::class.java)) {
+        @Entity
+        class State(
+                @Column
+                var id: String
+        ) : PersistentState()
+    }
+
+    lateinit var hikariProperties: Properties
+    lateinit var dataSource: DataSource
+
+    @Before
+    fun setUp() {
+        hikariProperties = MockServices.makeTestDataSourceProperties()
+        dataSource = DataSourceFactory.createDataSource(hikariProperties)
+    }
+
+    private fun createSchemaMigration(schemasToMigrate: Set<MappedSchema>, forceThrowOnMissingMigration: Boolean): SchemaMigration {
+        val databaseConfig = DatabaseConfig()
+        return SchemaMigration(schemasToMigrate, dataSource, databaseConfig, null, null,
+                TestIdentity(ALICE_NAME, 70).name, forceThrowOnMissingMigration)
+    }
+
+    @Test
+    fun `test that an error is thrown when forceThrowOnMissingMigration is set and a mapped schema is missing a migration`() {
+        assertThatThrownBy {
+            createSchemaMigration(setOf(GoodSchema), true)
+                .nodeStartup(dataSource.connection.use { DBCheckpointStorage().getCheckpointCount(it) != 0L })
+        }.isInstanceOf(MissingMigrationException::class.java)
+    }
+
+    @Test
+    fun `test that an error is not thrown when forceThrowOnMissingMigration is not set and a mapped schema is missing a migration`() {
+        assertDoesNotThrow {
+            createSchemaMigration(setOf(GoodSchema), false)
+                    .nodeStartup(dataSource.connection.use { DBCheckpointStorage().getCheckpointCount(it) != 0L })
+        }
+    }
+
+    @Test
+    fun `test that there are no missing migrations for the node`() {
+        assertDoesNotThrow("This test failure indicates " +
+                "a new table has been added to the node without the appropriate migration scripts being present") {
+            createSchemaMigration(NodeSchemaService().internalSchemas(), false)
+                    .nodeStartup(dataSource.connection.use { DBCheckpointStorage().getCheckpointCount(it) != 0L })
+        }
+    }
+
+}