From b1334241c785423ded28c58f3b601c167fdcc710 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Thu, 1 Jul 2021 15:17:39 -0400 Subject: [PATCH] change out-of-order flag to switch-order flag so the default behaviour reverses (#2773) * change out-of-order flag to switch-order flag so the default behaviour reverses * change log --- .../cli/BaseFlywayMigrateDatabaseCommand.java | 18 +++++++++++------- .../5_5_0/2773-flyway-out-of-order.yaml | 5 +++++ .../ca/uhn/fhir/jpa/migrate/BaseMigrator.java | 12 ++++++------ .../uhn/fhir/jpa/migrate/FlywayMigrator.java | 5 +++-- .../uhn/fhir/jpa/migrate/SchemaMigrator.java | 16 ++++++---------- .../fhir/jpa/migrate/SchemaMigratorTest.java | 3 ++- 6 files changed, 33 insertions(+), 26 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2773-flyway-out-of-order.yaml diff --git a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseFlywayMigrateDatabaseCommand.java b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseFlywayMigrateDatabaseCommand.java index 914530c21c3..47c2bd68bcb 100644 --- a/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseFlywayMigrateDatabaseCommand.java +++ b/hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/BaseFlywayMigrateDatabaseCommand.java @@ -20,7 +20,11 @@ package ca.uhn.fhir.cli; * #L% */ -import ca.uhn.fhir.jpa.migrate.*; +import ca.uhn.fhir.jpa.migrate.BaseMigrator; +import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; +import ca.uhn.fhir.jpa.migrate.FlywayMigrator; +import ca.uhn.fhir.jpa.migrate.MigrationTaskSkipper; +import ca.uhn.fhir.jpa.migrate.TaskOnlyMigrator; import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.Options; @@ -30,9 +34,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.Arrays; +import java.util.Collection; import java.util.List; import java.util.Set; -import java.util.*; import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.defaultString; @@ -46,7 +50,7 @@ public abstract class BaseFlywayMigrateDatabaseCommand extends B public static final String MIGRATE_DATABASE = "migrate-database"; public static final String NO_COLUMN_SHRINK = "no-column-shrink"; public static final String DONT_USE_FLYWAY = "dont-use-flyway"; - public static final String OUT_OF_ORDER_PERMITTED = "out-of-order-permitted"; + public static final String STRICT_ORDER = "strict-order"; public static final String SKIP_VERSIONS = "skip-versions"; private Set myFlags; private String myMigrationTableName; @@ -79,8 +83,8 @@ public abstract class BaseFlywayMigrateDatabaseCommand extends B addRequiredOption(retVal, "p", "password", "Password", "The JDBC database password"); addRequiredOption(retVal, "d", "driver", "Driver", "The database driver to use (Options are " + driverOptions() + ")"); addOptionalOption(retVal, "x", "flags", "Flags", "A comma-separated list of any specific migration flags (these flags are version specific, see migrator documentation for details)"); - addOptionalOption(retVal, null, DONT_USE_FLYWAY,false, "If this option is set, the migrator will not use FlywayDB for migration. This setting should only be used if you are trying to migrate a legacy database platform that is not supported by FlywayDB."); - addOptionalOption(retVal, null, OUT_OF_ORDER_PERMITTED,false, "If this option is set, the migrator will permit migration tasks to be run out of order. It shouldn't be required in most cases, however may be the solution if you see the error message 'Detected resolved migration not applied to database'."); + addOptionalOption(retVal, null, DONT_USE_FLYWAY, false, "If this option is set, the migrator will not use FlywayDB for migration. This setting should only be used if you are trying to migrate a legacy database platform that is not supported by FlywayDB."); + addOptionalOption(retVal, null, STRICT_ORDER, false, "If this option is set, the migrator will require migration tasks to be performed in order."); addOptionalOption(retVal, null, NO_COLUMN_SHRINK, false, "If this flag is set, the system will not attempt to reduce the length of columns. This is useful in environments with a lot of existing data, where shrinking a column can take a very long time."); addOptionalOption(retVal, null, SKIP_VERSIONS, "Versions", "A comma separated list of schema versions to skip. E.g. 4_1_0.20191214.2,4_1_0.20191214.4"); @@ -115,7 +119,7 @@ public abstract class BaseFlywayMigrateDatabaseCommand extends B .collect(Collectors.toSet()); boolean dontUseFlyway = theCommandLine.hasOption(BaseFlywayMigrateDatabaseCommand.DONT_USE_FLYWAY); - boolean outOfOrderPermitted = theCommandLine.hasOption(BaseFlywayMigrateDatabaseCommand.OUT_OF_ORDER_PERMITTED); + boolean strictOrder = theCommandLine.hasOption(BaseFlywayMigrateDatabaseCommand.STRICT_ORDER); BaseMigrator migrator; if (dontUseFlyway || dryRun) { @@ -131,7 +135,7 @@ public abstract class BaseFlywayMigrateDatabaseCommand extends B migrator.setDriverType(driverType); migrator.setDryRun(dryRun); migrator.setNoColumnShrink(noColumnShrink); - migrator.setOutOfOrderPermitted(outOfOrderPermitted); + migrator.setStrictOrder(strictOrder); String skipVersions = theCommandLine.getOptionValue(BaseFlywayMigrateDatabaseCommand.SKIP_VERSIONS); addTasks(migrator, skipVersions); migrator.migrate(); diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2773-flyway-out-of-order.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2773-flyway-out-of-order.yaml new file mode 100644 index 00000000000..1a6f087a4d7 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_5_0/2773-flyway-out-of-order.yaml @@ -0,0 +1,5 @@ +--- +type: change +issue: 2773 +title: "Flyway migration used to enforce order by default. This has been changed so now the default behaviour is +out of order migrations are permitted. Strict order can be enforced via the new strict-order flag if required." 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 399cf0c6f0c..9991b57859d 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 @@ -34,11 +34,11 @@ import java.util.Objects; public abstract class BaseMigrator implements IMigrator { private boolean myDryRun; private boolean myNoColumnShrink; - private boolean myOutOfOrderPermitted; + private final List myExecutedStatements = new ArrayList<>(); private boolean mySchemaWasInitialized; private DriverTypeEnum myDriverType; private DataSource myDataSource; - private List myExecutedStatements = new ArrayList<>(); + private boolean myStrictOrder; private List myCallbacks = Collections.emptyList(); @Nonnull @@ -83,12 +83,12 @@ public abstract class BaseMigrator implements IMigrator { myDriverType = theDriverType; } - public boolean isOutOfOrderPermitted() { - return myOutOfOrderPermitted; + public boolean isStrictOrder() { + return myStrictOrder; } - public void setOutOfOrderPermitted(boolean theOutOfOrderPermitted) { - myOutOfOrderPermitted = theOutOfOrderPermitted; + public void setStrictOrder(boolean theStrictOrder) { + myStrictOrder = theStrictOrder; } public void addExecutedStatements(List theExecutedStatements) { 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 32337c54ef3..1ed7b1b1971 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 final List myTasks = new ArrayList<>(); public FlywayMigrator(String theMigrationTableName, DataSource theDataSource, DriverTypeEnum theDriverType) { this(theMigrationTableName); @@ -76,7 +76,8 @@ public class FlywayMigrator extends BaseMigrator { .table(myMigrationTableName) .dataSource(theConnectionProperties.getDataSource()) .baselineOnMigrate(true) - .outOfOrder(isOutOfOrderPermitted()) + // By default, migrations are allowed to be run out of order. You can enforce strict order by setting strictOrder=true. + .outOfOrder(!isStrictOrder()) .javaMigrations(myTasks.toArray(new JavaMigration[0])) .callbacks(getCallbacks().toArray(new Callback[0])) .load(); diff --git a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/SchemaMigrator.java b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/SchemaMigrator.java index 8dcd1e31294..bd844e64149 100644 --- a/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/SchemaMigrator.java +++ b/hapi-fhir-jpaserver-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/SchemaMigrator.java @@ -47,7 +47,7 @@ public class SchemaMigrator { private final String myMigrationTableName; private final List myMigrationTasks; private boolean myDontUseFlyway; - private boolean myOutOfOrderPermitted; + private boolean myStrictOrder; private DriverTypeEnum myDriverType; private List myCallbacks = Collections.emptyList(); @@ -60,11 +60,7 @@ public class SchemaMigrator { myMigrationTableName = theMigrationTableName; myMigrationTasks = theMigrationTasks; - if (jpaProperties.containsKey(AvailableSettings.HBM2DDL_AUTO) && "update".equals(jpaProperties.getProperty(AvailableSettings.HBM2DDL_AUTO))) { - mySkipValidation = true; - } else { - mySkipValidation = false; - } + mySkipValidation = jpaProperties.containsKey(AvailableSettings.HBM2DDL_AUTO) && "update".equals(jpaProperties.getProperty(AvailableSettings.HBM2DDL_AUTO)); } public void setCallbacks(List theCallbacks) { @@ -76,8 +72,8 @@ public class SchemaMigrator { myDontUseFlyway = theDontUseFlyway; } - public void setOutOfOrderPermitted(boolean theOutOfOrderPermitted) { - myOutOfOrderPermitted = theOutOfOrderPermitted; + public void setStrictOrder(boolean theStrictOrder) { + myStrictOrder = theStrictOrder; } public void validate() { @@ -98,7 +94,7 @@ public class SchemaMigrator { ourLog.info("Database schema confirmed at expected version " + getCurrentVersion(migrationInfo.get())); } } catch (SQLException e) { - throw new ConfigurationException("Unable to connect to " + myDataSource.toString(), e); + throw new ConfigurationException("Unable to connect to " + myDataSource, e); } } @@ -125,7 +121,7 @@ public class SchemaMigrator { migrator.setDataSource(myDataSource); } else { migrator = new FlywayMigrator(myMigrationTableName, myDataSource, myDriverType); - migrator.setOutOfOrderPermitted(myOutOfOrderPermitted); + migrator.setStrictOrder(myStrictOrder); } migrator.addTasks(myMigrationTasks); migrator.setCallbacks(myCallbacks); diff --git a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/SchemaMigratorTest.java b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/SchemaMigratorTest.java index 0b54d0c5f1d..2eb015b0535 100644 --- a/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/SchemaMigratorTest.java +++ b/hapi-fhir-jpaserver-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/SchemaMigratorTest.java @@ -73,6 +73,7 @@ public class SchemaMigratorTest extends BaseTest { schemaMigrator.migrate(); schemaMigrator = createSchemaMigrator("SOMETABLE", "create table SOMEOTHERTABLE (PID bigint not null, TEXTCOL varchar(255))", "1"); + schemaMigrator.setStrictOrder(true); try { schemaMigrator.migrate(); @@ -80,7 +81,7 @@ public class SchemaMigratorTest extends BaseTest { } catch (FlywayException e) { assertThat(e.getMessage(), containsString("Detected resolved migration not applied to database: 1.1")); } - schemaMigrator.setOutOfOrderPermitted(true); + schemaMigrator.setStrictOrder(false); schemaMigrator.migrate(); }