From 50ae07acaa57aa36f99ec052f7cb5aa2509dcd8b Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 9 Jul 2020 20:18:54 -0400 Subject: [PATCH] stub out all migrations if an empty schema was initialized (#1975) --- .../ca/uhn/fhir/jpa/migrate/BaseMigrator.java | 10 ++ ...igration.java => FlywayMigrationTask.java} | 24 +++- .../uhn/fhir/jpa/migrate/FlywayMigrator.java | 6 +- .../fhir/jpa/migrate/taskdef/BaseTask.java | 4 + .../migrate/taskdef/InitializeSchemaTask.java | 10 ++ .../tasks/HapiFhirJpaMigrationTasks.java | 2 +- .../tasks/SchemaInitializationProvider.java | 10 +- .../api/ISchemaInitializationProvider.java | 4 +- .../jpa/migrate/FlywayMigrationTaskTest.java | 120 ++++++++++++++++++ .../taskdef/InitializeSchemaTaskTest.java | 5 + 10 files changed, 184 insertions(+), 11 deletions(-) rename hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/{FlywayMigration.java => FlywayMigrationTask.java} (74%) create mode 100644 hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/FlywayMigrationTaskTest.java diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/BaseMigrator.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/BaseMigrator.java index d2f4332078f..d3795ae2596 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/BaseMigrator.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/BaseMigrator.java @@ -35,6 +35,7 @@ public abstract class BaseMigrator implements IMigrator { private boolean myDryRun; private boolean myNoColumnShrink; private boolean myOutOfOrderPermitted; + private boolean mySchemaWasInitialized; private DriverTypeEnum myDriverType; private DataSource myDataSource; private List myExecutedStatements = new ArrayList<>(); @@ -111,4 +112,13 @@ public abstract class BaseMigrator implements IMigrator { } return statementBuilder; } + + public boolean isSchemaWasInitialized() { + return mySchemaWasInitialized; + } + + public BaseMigrator setSchemaWasInitialized(boolean theSchemaWasInitialized) { + mySchemaWasInitialized = theSchemaWasInitialized; + return this; + } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigration.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigrationTask.java similarity index 74% rename from hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigration.java rename to hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigrationTask.java index f143de54a64..28b439180e0 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigration.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/FlywayMigrationTask.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.migrate; */ import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask; +import ca.uhn.fhir.jpa.migrate.taskdef.InitializeSchemaTask; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import org.flywaydb.core.api.MigrationVersion; import org.flywaydb.core.api.migration.Context; @@ -32,14 +33,14 @@ import java.sql.SQLException; import static org.apache.commons.lang3.StringUtils.isBlank; -public class FlywayMigration implements JavaMigration { - private static final Logger ourLog = LoggerFactory.getLogger(FlywayMigration.class); +public class FlywayMigrationTask implements JavaMigration { + private static final Logger ourLog = LoggerFactory.getLogger(FlywayMigrationTask.class); private final BaseTask myTask; private final FlywayMigrator myFlywayMigrator; private DriverTypeEnum.ConnectionProperties myConnectionProperties; - public FlywayMigration(BaseTask theTask, FlywayMigrator theFlywayMigrator) { + public FlywayMigrationTask(BaseTask theTask, FlywayMigrator theFlywayMigrator) { myTask = theTask; myFlywayMigrator = theFlywayMigrator; } @@ -76,8 +77,7 @@ public class FlywayMigration implements JavaMigration { myTask.setNoColumnShrink(myFlywayMigrator.isNoColumnShrink()); myTask.setConnectionProperties(myConnectionProperties); try { - myTask.execute(); - myFlywayMigrator.addExecutedStatements(myTask.getExecutedStatements()); + executeTask(); } catch (SQLException e) { String description = myTask.getDescription(); if (isBlank(description)) { @@ -88,6 +88,20 @@ public class FlywayMigration implements JavaMigration { } } + private void executeTask() throws SQLException { + if (myFlywayMigrator.isSchemaWasInitialized() && !(myTask instanceof InitializeSchemaTask)) { + // Empty schema was initialized, stub out this non-schema-init task since we're starting with a fully migrated schema + myTask.setDoNothing(true); + } + myTask.execute(); + if (myTask.initializedSchema()) { + ourLog.info("Empty schema was Initialized. Stubbing out all following migration tasks that are not Schema Initializations."); + myFlywayMigrator.setSchemaWasInitialized(true); + } + + myFlywayMigrator.addExecutedStatements(myTask.getExecutedStatements()); + } + public void setConnectionProperties(DriverTypeEnum.ConnectionProperties theConnectionProperties) { myConnectionProperties = theConnectionProperties; } 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 c71c5d9b43d..f423667f783 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 @@ -40,7 +40,7 @@ public class FlywayMigrator extends BaseMigrator { private static final Logger ourLog = LoggerFactory.getLogger(FlywayMigrator.class); private final String myMigrationTableName; - private List myTasks = new ArrayList<>(); + private List myTasks = new ArrayList<>(); public FlywayMigrator(String theMigrationTableName, DataSource theDataSource, DriverTypeEnum theDriverType) { this(theMigrationTableName); @@ -53,7 +53,7 @@ public class FlywayMigrator extends BaseMigrator { } public void addTask(BaseTask theTask) { - myTasks.add(new FlywayMigration(theTask, this)); + myTasks.add(new FlywayMigrationTask(theTask, this)); } @Override @@ -80,7 +80,7 @@ public class FlywayMigrator extends BaseMigrator { .javaMigrations(myTasks.toArray(new JavaMigration[0])) .callbacks(getCallbacks().toArray(new Callback[0])) .load(); - for (FlywayMigration task : myTasks) { + for (FlywayMigrationTask task : myTasks) { task.setConnectionProperties(theConnectionProperties); } return flyway; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java index 39b666a8e61..e47787244a8 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/BaseTask.java @@ -230,6 +230,10 @@ public abstract class BaseTask { protected abstract void generateEquals(EqualsBuilder theBuilder, BaseTask theOtherObject); + public boolean initializedSchema() { + return false; + } + public static class ExecutedStatement { private final String mySql; private final List myArguments; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTask.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTask.java index 3664c4c271d..f58a62fadbc 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTask.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTask.java @@ -37,6 +37,7 @@ public class InitializeSchemaTask extends BaseTask { public static final String DESCRIPTION_PREFIX = "Initialize schema for "; private final ISchemaInitializationProvider mySchemaInitializationProvider; + private boolean myInitializedSchema; public InitializeSchemaTask(String theProductVersion, String theSchemaVersion, ISchemaInitializationProvider theSchemaInitializationProvider) { super(theProductVersion, theSchemaVersion); @@ -68,9 +69,18 @@ public class InitializeSchemaTask extends BaseTask { executeSql(null, nextSql); } + if (mySchemaInitializationProvider.canInitializeSchema()) { + myInitializedSchema = true; + } + logInfo(ourLog, "{} schema for {} initialized successfully", driverType, mySchemaInitializationProvider.getSchemaDescription()); } + @Override + public boolean initializedSchema() { + return myInitializedSchema; + } + @Override protected void generateEquals(EqualsBuilder theBuilder, BaseTask theOtherObject) { InitializeSchemaTask otherObject = (InitializeSchemaTask) theOtherObject; diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 67789986d8e..2d44c2b84ce 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -1139,7 +1139,7 @@ public class HapiFhirJpaMigrationTasks extends BaseMigrationTasks { protected void init330() { // 20180114 - 20180329 Builder version = forVersion(VersionEnum.V3_3_0); - version.initializeSchema("20180115.0", new SchemaInitializationProvider("HAPI FHIR", "/ca/uhn/hapi/fhir/jpa/docs/database", "HFJ_RESOURCE")); + version.initializeSchema("20180115.0", new SchemaInitializationProvider("HAPI FHIR", "/ca/uhn/hapi/fhir/jpa/docs/database", "HFJ_RESOURCE", true)); Builder.BuilderWithTableName hfjResource = version.onTable("HFJ_RESOURCE"); version.startSectionWithMessage("Starting work on table: " + hfjResource.getTableName()); diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/SchemaInitializationProvider.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/SchemaInitializationProvider.java index 262dbb576af..d0cfbd7e73e 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/SchemaInitializationProvider.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/SchemaInitializationProvider.java @@ -41,15 +41,18 @@ public class SchemaInitializationProvider implements ISchemaInitializationProvid private String mySchemaDescription; private final String mySchemaExistsIndicatorTable; + private final boolean myCanInitializeSchema; /** * @param theSchemaFileClassPath pathname to script used to initialize schema * @param theSchemaExistsIndicatorTable a table name we can use to determine if this schema has already been initialized + * @param theCanInitializeSchema this is a "root" schema initializer that creates the primary tables used by this app */ - public SchemaInitializationProvider(String theSchemaDescription, String theSchemaFileClassPath, String theSchemaExistsIndicatorTable) { + public SchemaInitializationProvider(String theSchemaDescription, String theSchemaFileClassPath, String theSchemaExistsIndicatorTable, boolean theCanInitializeSchema) { mySchemaDescription = theSchemaDescription; mySchemaFileClassPath = theSchemaFileClassPath; mySchemaExistsIndicatorTable = theSchemaExistsIndicatorTable; + myCanInitializeSchema = theCanInitializeSchema; } @Override @@ -129,5 +132,10 @@ public class SchemaInitializationProvider implements ISchemaInitializationProvid mySchemaDescription = theSchemaDescription; return this; } + + @Override + public boolean canInitializeSchema() { + return myCanInitializeSchema; + } } diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/ISchemaInitializationProvider.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/ISchemaInitializationProvider.java index 1be30796e25..dc7ef6b6c02 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/ISchemaInitializationProvider.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/api/ISchemaInitializationProvider.java @@ -30,7 +30,9 @@ public interface ISchemaInitializationProvider { String getSchemaExistsIndicatorTable(); - String getSchemaDescription(); + String getSchemaDescription(); ISchemaInitializationProvider setSchemaDescription(String theSchemaDescription); + + boolean canInitializeSchema(); } diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/FlywayMigrationTaskTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/FlywayMigrationTaskTest.java new file mode 100644 index 00000000000..1cedd099139 --- /dev/null +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/FlywayMigrationTaskTest.java @@ -0,0 +1,120 @@ +package ca.uhn.fhir.jpa.migrate; + +import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask; +import ca.uhn.fhir.jpa.migrate.taskdef.InitializeSchemaTask; +import ca.uhn.fhir.jpa.migrate.tasks.api.ISchemaInitializationProvider; +import org.apache.commons.lang3.builder.EqualsBuilder; +import org.apache.commons.lang3.builder.HashCodeBuilder; +import org.flywaydb.core.api.migration.Context; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.sql.SQLException; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class FlywayMigrationTaskTest { + @Mock + private FlywayMigrator myFlywayMigrator; + @Mock + private Context myContext; + + TestTask myTestTask = new TestTask(); + + @Test + public void schemaInitializedStubsFollowingMigration() { + when(myFlywayMigrator.isSchemaWasInitialized()).thenReturn(true); + FlywayMigrationTask task = new FlywayMigrationTask(myTestTask, myFlywayMigrator); + task.migrate(myContext); + assertTrue(myTestTask.isDoNothing()); + } + + @Test + public void schemaNotInitializedStubsFollowingMigration() { + when(myFlywayMigrator.isSchemaWasInitialized()).thenReturn(false); + FlywayMigrationTask task = new FlywayMigrationTask(myTestTask, myFlywayMigrator); + task.migrate(myContext); + assertFalse(myTestTask.isDoNothing()); + } + + @Test + public void schemaInitializedStubsFollowingMigrationExceptInitSchemaTask() { + when(myFlywayMigrator.isSchemaWasInitialized()).thenReturn(true); + InitializeSchemaTask initSchemaTask = new TestInitializeSchemaTask(false); + FlywayMigrationTask task = new FlywayMigrationTask(initSchemaTask, myFlywayMigrator); + task.migrate(myContext); + assertFalse(myTestTask.isDoNothing()); + } + + @Test + public void schemaInitializedSetsInitializedFlag() { + InitializeSchemaTask initSchemaTask = new TestInitializeSchemaTask(true); + FlywayMigrationTask task = new FlywayMigrationTask(initSchemaTask, myFlywayMigrator); + task.migrate(myContext); + verify(myFlywayMigrator, times(1)).setSchemaWasInitialized(true); + } + + + @Test + public void nonInitSchemaInitializedSetsInitializedFlag() { + InitializeSchemaTask initSchemaTask = new TestInitializeSchemaTask(false); + FlywayMigrationTask task = new FlywayMigrationTask(initSchemaTask, myFlywayMigrator); + task.migrate(myContext); + verify(myFlywayMigrator, never()).setSchemaWasInitialized(true); + } + + // Can't use @Mock since BaseTask.equals is final + private class TestTask extends BaseTask { + protected TestTask() { + super("1", "1"); + } + + @Override + public void validate() { + // do nothing + } + + @Override + protected void doExecute() throws SQLException { + // do nothing + } + + @Override + protected void generateHashCode(HashCodeBuilder theBuilder) { + // do nothing + } + + @Override + protected void generateEquals(EqualsBuilder theBuilder, BaseTask theOtherObject) { + // do nothing + } + } + + private class TestInitializeSchemaTask extends InitializeSchemaTask { + private final boolean myInitializedSchema; + + public TestInitializeSchemaTask(boolean theInitializedSchema) { + super("1", "1", mock(ISchemaInitializationProvider.class)); + myInitializedSchema = theInitializedSchema; + } + + @Override + public void execute() throws SQLException { + // nothing + } + + @Override + public boolean initializedSchema() { + return myInitializedSchema; + } + } +} diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTaskTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTaskTest.java index bdf2a8c2b5e..7a469c372b5 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTaskTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTaskTest.java @@ -56,6 +56,11 @@ public class InitializeSchemaTaskTest extends BaseTest { return this; } + @Override + public boolean canInitializeSchema() { + return false; + } + @Override public boolean equals(Object theO) { if (this == theO) return true;