From f95e4d387d2f3b26f5e01dfc76165871e51cfe7a Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 31 Oct 2019 14:56:55 -0400 Subject: [PATCH] added schema migration validation --- .../cli/HapiFlywayMigrateDatabaseCommand.java | 3 +- .../uhn/fhir/jpa/migrate/DriverTypeEnum.java | 13 +++++ .../uhn/fhir/jpa/migrate/FlywayMigrator.java | 27 ++++++++- .../fhir/jpa/migrate/MigrationValidator.java | 58 +++++++++++++++++++ .../jpa/migrate/MigrationValidatorTest.java | 32 ++++++++++ .../taskdef/AddIdGeneratorTaskTest.java | 8 +-- .../fhir/jpa/migrate/taskdef/BaseTest.java | 39 ++++++++----- .../jpa/migrate/taskdef/DropTableTest.java | 9 +-- 8 files changed, 163 insertions(+), 26 deletions(-) create mode 100644 hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/MigrationValidator.java create mode 100644 hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/MigrationValidatorTest.java diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommand.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommand.java index 49b04819dd9..e9951255357 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommand.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/HapiFlywayMigrateDatabaseCommand.java @@ -42,7 +42,8 @@ public class HapiFlywayMigrateDatabaseCommand extends BaseFlywayMigrateDatabaseC @Override protected void addTasks(FlywayMigrator theMigrator) { + // FIXME KHS if no shrink columns set, apply null migration with same id List> tasks = new HapiFhirJpaMigrationTasks(getFlags()).getAllTasks(VersionEnum.values()); - tasks.forEach(theMigrator::addTask); + theMigrator.addTasks(tasks); } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/DriverTypeEnum.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/DriverTypeEnum.java index d676d0117ad..e4124cee7ec 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/DriverTypeEnum.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/DriverTypeEnum.java @@ -66,6 +66,19 @@ public enum DriverTypeEnum { myDerby = theDerby; } + public String getDriverClassName() { + return myDriverClassName; + } + + public static DriverTypeEnum fromDriverClassName(String theDriverClassName) { + for (DriverTypeEnum driverTypeEnum : DriverTypeEnum.values()) { + if (driverTypeEnum.myDriverClassName.equals(theDriverClassName)) { + return driverTypeEnum; + } + } + return null; + } + public ConnectionProperties newConnectionProperties(String theUrl, String theUsername, String thePassword) { Driver driver; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigrator.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigrator.java index 9a7983d88ec..981bdf99054 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigrator.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigrator.java @@ -21,12 +21,14 @@ package ca.uhn.fhir.jpa.migrate; */ import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask; +import org.apache.commons.dbcp2.BasicDataSource; import org.flywaydb.core.Flyway; import org.flywaydb.core.api.MigrationInfoService; import org.flywaydb.core.api.migration.JavaMigration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.sql.Driver; import java.util.ArrayList; import java.util.List; @@ -42,6 +44,23 @@ public class FlywayMigrator { private boolean myDryRun; private boolean myNoColumnShrink; + public FlywayMigrator() {} + + public FlywayMigrator(BasicDataSource theDataSource) { + myConnectionUrl = theDataSource.getUrl(); + myUsername = theDataSource.getUsername(); + myPassword = theDataSource.getPassword(); + String driverClassName = theDataSource.getDriverClassName(); + if (driverClassName == null) { + ourLog.error(this.getClass().getSimpleName() + " constructed without a database driver"); + } else { + myDriverType = DriverTypeEnum.fromDriverClassName(driverClassName); + if (myDriverType == null) { + ourLog.error("Unknown driver class " + driverClassName); + } + } + } + public void setDriverType(DriverTypeEnum theDriverType) { myDriverType = theDriverType; } @@ -117,11 +136,13 @@ public class FlywayMigrator { return myNoColumnShrink; } - public boolean migrationRequired() { + public MigrationInfoService getMigrationInfo() { + if (myDriverType == null) { + return null; + } try (DriverTypeEnum.ConnectionProperties connectionProperties = myDriverType.newConnectionProperties(myConnectionUrl, myUsername, myPassword)) { Flyway flyway = initFlyway(connectionProperties); - MigrationInfoService info = flyway.info(); - return info.pending().length > 0; + return flyway.info(); } } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/MigrationValidator.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/MigrationValidator.java new file mode 100644 index 00000000000..a0797220ae3 --- /dev/null +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/MigrationValidator.java @@ -0,0 +1,58 @@ +package ca.uhn.fhir.jpa.migrate; + +import ca.uhn.fhir.context.ConfigurationException; +import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask; +import ca.uhn.fhir.jpa.subscription.module.subscriber.websocket.WebsocketConnectionValidator; +import org.apache.commons.dbcp2.BasicDataSource; +import org.flywaydb.core.api.MigrationInfo; +import org.flywaydb.core.api.MigrationInfoService; +import org.flywaydb.core.api.MigrationVersion; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.sql.Connection; +import java.sql.SQLException; +import java.util.List; + +public class MigrationValidator { + private static final Logger ourLog = LoggerFactory.getLogger(WebsocketConnectionValidator.class); + + private final BasicDataSource myDataSource; + private final FlywayMigrator myMigrator; + + public MigrationValidator(BasicDataSource theDataSource, List> theMigrationTasks) { + myDataSource = theDataSource; + myMigrator = new FlywayMigrator(theDataSource); + myMigrator.addTasks(theMigrationTasks); + } + + public void validate() { + try (Connection connection = myDataSource.getConnection()) { + MigrationInfoService migrationInfo = myMigrator.getMigrationInfo(); + if (migrationInfo.pending().length > 0) { + throw new ConfigurationException("The database schema for " + connection.getCatalog() + " is out of date. " + + "Current database schema version is " + getCurrentVersion(migrationInfo) + ". Schema version required by application is " + + getLastVersion(migrationInfo) + ". Please run the database migrator."); + } + ourLog.info("Database schema confirmed at expected version " + getCurrentVersion(migrationInfo)); + } catch (SQLException e) { + throw new ConfigurationException("Unable to connect to " + myDataSource.toString(), e); + } + } + + private String getCurrentVersion(MigrationInfoService theMigrationInfo) { + MigrationInfo migrationInfo = theMigrationInfo.current(); + if (migrationInfo == null) { + return "unknown"; + } + return migrationInfo.getVersion().toString(); + } + + private String getLastVersion(MigrationInfoService theMigrationInfo) { + MigrationInfo[] pending = theMigrationInfo.pending(); + if (pending.length > 0) { + return pending[pending.length - 1].getVersion().toString(); + } + return "unknown"; + } +} diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/MigrationValidatorTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/MigrationValidatorTest.java new file mode 100644 index 00000000000..c1b88c8a515 --- /dev/null +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/MigrationValidatorTest.java @@ -0,0 +1,32 @@ +package ca.uhn.fhir.jpa.migrate; + +import ca.uhn.fhir.context.ConfigurationException; +import ca.uhn.fhir.jpa.migrate.taskdef.AddTableRawSqlTask; +import ca.uhn.fhir.jpa.migrate.taskdef.BaseTest; +import org.junit.Before; +import org.junit.Test; + +import java.util.Collections; + +import static org.junit.Assert.*; + +public class MigrationValidatorTest extends BaseTest { + + @Test + public void migrationRequired() { + AddTableRawSqlTask task = new AddTableRawSqlTask("1", "1"); + task.setTableName("SOMETABLE"); + task.addSql(DriverTypeEnum.H2_EMBEDDED, "create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255))"); + getMigrator().addTask(task); + + MigrationValidator migrationValidator = new MigrationValidator(getDataSource(), Collections.singletonList(task)); + try { + migrationValidator.validate(); + fail(); + } catch (ConfigurationException e) { + assertEquals("The database schema for " + getDatabaseName() + " is out of date. Current database schema version is unknown. Schema version required by application is " + task.getFlywayVersion() + ". Please run the database migrator.", e.getMessage()); + } + getMigrator().migrate(); + migrationValidator.validate(); + } +} diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIdGeneratorTaskTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIdGeneratorTaskTest.java index 1e3b341c8ae..089bf089df8 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIdGeneratorTaskTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/AddIdGeneratorTaskTest.java @@ -19,15 +19,15 @@ public class AddIdGeneratorTaskTest extends BaseTest { public void testAddIdGenerator() throws SQLException { assertThat(JdbcUtils.getSequenceNames(getConnectionProperties()), empty()); - MyMigrationTasks migrator = new MyMigrationTasks("123456.7"); - getMigrator().addTasks(migrator.getTasks(VersionEnum.V3_3_0, VersionEnum.V3_6_0)); + MyMigrationTasks migrationTasks = new MyMigrationTasks("123456.7"); + getMigrator().addTasks(migrationTasks.getTasks(VersionEnum.V3_3_0, VersionEnum.V3_6_0)); getMigrator().migrate(); assertThat(JdbcUtils.getSequenceNames(getConnectionProperties()), containsInAnyOrder("SEQ_FOO")); // Second time, should produce no action - migrator = new MyMigrationTasks("123456.8"); - getMigrator().addTasks(migrator.getTasks(VersionEnum.V3_3_0, VersionEnum.V3_6_0)); + migrationTasks = new MyMigrationTasks("123456.8"); + getMigrator().addTasks(migrationTasks.getTasks(VersionEnum.V3_3_0, VersionEnum.V3_6_0)); getMigrator().migrate(); assertThat(JdbcUtils.getSequenceNames(getConnectionProperties()), containsInAnyOrder("SEQ_FOO")); diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java index 772cf65c933..fe7acfa3f63 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTest.java @@ -3,6 +3,7 @@ package ca.uhn.fhir.jpa.migrate.taskdef; import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; import ca.uhn.fhir.jpa.migrate.FlywayMigrator; import ca.uhn.fhir.jpa.migrate.Migrator; +import org.apache.commons.dbcp2.BasicDataSource; import org.intellij.lang.annotations.Language; import org.junit.After; import org.junit.Before; @@ -13,10 +14,12 @@ import java.util.Map; public class BaseTest { + private static final String DATABASE_NAME = "DATABASE"; private static int ourDatabaseUrl = 0; private String myUrl; private FlywayMigrator myMigrator; private DriverTypeEnum.ConnectionProperties myConnectionProperties; + private final BasicDataSource myDataSource = new BasicDataSource(); public String getUrl() { return myUrl; @@ -26,6 +29,28 @@ public class BaseTest { return myConnectionProperties; } + @Before() + public void before() { + org.h2.Driver.class.toString(); + ++ourDatabaseUrl; + myUrl = "jdbc:h2:mem:" + getDatabaseName(); + + myConnectionProperties = DriverTypeEnum.H2_EMBEDDED.newConnectionProperties(myUrl, "SA", "SA"); + myDataSource.setUrl(myUrl); + myDataSource.setUsername("SA"); + myDataSource.setPassword("SA"); + myDataSource.setDriverClassName(DriverTypeEnum.H2_EMBEDDED.getDriverClassName()); + myMigrator = new FlywayMigrator(myDataSource); + } + + protected String getDatabaseName() { + return DATABASE_NAME + ourDatabaseUrl; + } + + protected BasicDataSource getDataSource() { + return myDataSource; + } + @After public void resetMigrationVersion() { executeSql("DELETE from \"flyway_schema_history\" where \"installed_rank\" > 0"); @@ -53,19 +78,5 @@ public class BaseTest { myConnectionProperties.close(); } - @Before() - public void before() { - org.h2.Driver.class.toString(); - - myUrl = "jdbc:h2:mem:database" + (ourDatabaseUrl++); - - myConnectionProperties = DriverTypeEnum.H2_EMBEDDED.newConnectionProperties(myUrl, "SA", "SA"); - - myMigrator = new FlywayMigrator(); - myMigrator.setConnectionUrl(myUrl); - myMigrator.setDriverType(DriverTypeEnum.H2_EMBEDDED); - myMigrator.setUsername("SA"); - myMigrator.setPassword("SA"); - } } diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTest.java index 232f772217c..e7c6ef807aa 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTest.java @@ -4,8 +4,9 @@ import ca.uhn.fhir.jpa.migrate.JdbcUtils; import org.junit.Test; import java.sql.SQLException; +import java.util.Collections; -import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.*; import static org.hamcrest.core.IsNot.not; import static org.junit.Assert.*; @@ -58,7 +59,7 @@ public class DropTableTest extends BaseTest { } @Test - public void testFlywayMigrationRequired() throws SQLException { + public void testFlywayGetMigrationInfo() throws SQLException { executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255))"); DropTableTask task = new DropTableTask("1", "1"); @@ -67,9 +68,9 @@ public class DropTableTest extends BaseTest { assertThat(JdbcUtils.getTableNames(getConnectionProperties()), (hasItems("SOMETABLE"))); - assertTrue(getMigrator().migrationRequired()); + assertThat(getMigrator().getMigrationInfo().pending().length, greaterThan(0)); getMigrator().migrate(); - assertFalse(getMigrator().migrationRequired()); + assertThat(getMigrator().getMigrationInfo().pending().length, equalTo(0)); assertThat(JdbcUtils.getTableNames(getConnectionProperties()), not(hasItems("SOMETABLE"))); }