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

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<T extends Enum> 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<String> myFlags;
 	private String myMigrationTableName;
@@ -79,8 +83,8 @@ public abstract class BaseFlywayMigrateDatabaseCommand<T extends Enum> 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, 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, 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<T extends Enum> 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<T extends Enum> 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();

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."

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<BaseTask.ExecutedStatement> myExecutedStatements = new ArrayList<>();
 	private boolean mySchemaWasInitialized;
 	private DriverTypeEnum myDriverType;
 	private DataSource myDataSource;
-	private List<BaseTask.ExecutedStatement> myExecutedStatements = new ArrayList<>();
+	private boolean myStrictOrder;
 	private List<Callback> myCallbacks = Collections.emptyList();
 
 	@Nonnull
@@ -83,12 +83,12 @@ public abstract class BaseMigrator implements IMigrator {
 		myDriverType = theDriverType;
 	}
 
-	public boolean isOutOfOrderPermitted() {
+	public boolean isStrictOrder() {
-		return myOutOfOrderPermitted;
+		return myStrictOrder;
 	}
 
-	public void setOutOfOrderPermitted(boolean theOutOfOrderPermitted) {
+	public void setStrictOrder(boolean theStrictOrder) {
-		myOutOfOrderPermitted = theOutOfOrderPermitted;
+		myStrictOrder = theStrictOrder;
 	}
 
 	public void addExecutedStatements(List theExecutedStatements) {

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<FlywayMigrationTask> myTasks = new ArrayList<>();
+	private final List<FlywayMigrationTask> 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();

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<BaseTask> myMigrationTasks;
 	private boolean myDontUseFlyway;
-	private boolean myOutOfOrderPermitted;
+	private boolean myStrictOrder;
 	private DriverTypeEnum myDriverType;
 	private List<Callback> 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 = jpaProperties.containsKey(AvailableSettings.HBM2DDL_AUTO) && "update".equals(jpaProperties.getProperty(AvailableSettings.HBM2DDL_AUTO));
-			mySkipValidation = true;
-		} else {
-			mySkipValidation = false;
-		}
 	}
 
 	public void setCallbacks(List<Callback> theCallbacks) {
@@ -76,8 +72,8 @@ public class SchemaMigrator {
 		myDontUseFlyway = theDontUseFlyway;
 	}
 
-	public void setOutOfOrderPermitted(boolean theOutOfOrderPermitted) {
+	public void setStrictOrder(boolean theStrictOrder) {
-		myOutOfOrderPermitted = theOutOfOrderPermitted;
+		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);

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();
 	}