cleanup

hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/HapiMigrator.java

@@ -26,8 +26,6 @@ import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask;
 import ca.uhn.fhir.util.StopWatch;
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.lang3.Validate;
-import org.flywaydb.core.api.callback.Callback;
-import org.flywaydb.core.api.callback.Event;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -44,14 +42,12 @@ public class HapiMigrator {
 
 	private static final Logger ourLog = LoggerFactory.getLogger(HapiMigrator.class);
 	private List<BaseTask> myTasks = new ArrayList<>();
-	private final List<BaseTask.ExecutedStatement> myExecutedStatements = new ArrayList<>();
 	private boolean myDryRun;
 	private boolean myNoColumnShrink;
 	private final DriverTypeEnum myDriverType;
 	private final DataSource myDataSource;
 	private final HapiMigrationStorageSvc myHapiMigrationStorageSvc;
-	private List<Callback> myCallbacks = Collections.emptyList();
+	private List<IHapiMigrationCallback> myCallbacks = Collections.emptyList();
-	private int myChangesCount;
 
 	public HapiMigrator(DriverTypeEnum theDriverType, DataSource theDataSource, String theMigrationTableName) {
 		myDriverType = theDriverType;
@@ -84,10 +80,10 @@ public class HapiMigrator {
 	}
 
 
-	protected StringBuilder buildExecutedStatementsString() {
+	protected StringBuilder buildExecutedStatementsString(MigrationResult theMigrationResult) {
 		StringBuilder statementBuilder = new StringBuilder();
 		String lastTable = null;
-		for (BaseTask.ExecutedStatement next : myExecutedStatements) {
+		for (BaseTask.ExecutedStatement next : theMigrationResult.executedStatements) {
 			if (!Objects.equals(lastTable, next.getTableName())) {
 				statementBuilder.append("\n\n-- Table: ").append(next.getTableName()).append("\n");
 				lastTable = next.getTableName();
@@ -102,11 +98,13 @@ public class HapiMigrator {
 		return statementBuilder;
 	}
 
-	public void migrate() {
+	public MigrationResult migrate() {
 		ourLog.info("Loaded {} migration tasks", myTasks.size());
 		List<BaseTask> newTasks = myHapiMigrationStorageSvc.diff(myTasks);
 		ourLog.info("{} of these {} migration tasks are new.  Executing them now.", newTasks.size(), myTasks.size());
 
+		MigrationResult retval = new MigrationResult();
+
 		try (DriverTypeEnum.ConnectionProperties connectionProperties = getDriverType().newConnectionProperties(getDataSource())) {
 
 			for (BaseTask next : newTasks) {
@@ -122,15 +120,14 @@ public class HapiMigrator {
 					} else {
 						ourLog.info("Executing {} {}", next.getMigrationVersion(), next.getDescription());
 					}
-					// WIP KHS replace with different callback probably a BaseTask consumer
+					preExecute(next);
-					myCallbacks.forEach(action -> action.handle(Event.BEFORE_EACH_MIGRATE, null));
-					// WIP KHS break up
 					next.execute();
 					postExecute(next, sw, true);
-					myChangesCount += next.getChangesCount();
+					retval.changes += next.getChangesCount();
-					myExecutedStatements.addAll(next.getExecutedStatements());
+					retval.executedStatements.addAll(next.getExecutedStatements());
-
+					retval.succeededTasks.add(next);
 				} catch (SQLException e) {
+					retval.failedTasks.add(next);
 					postExecute(next, sw, false);
 					String description = next.getDescription();
 					if (isBlank(description)) {
@@ -142,12 +139,19 @@ public class HapiMigrator {
 			}
 		}
 
-		ourLog.info("Completed executing {} migration tasks resulting in {} changes", myTasks.size(), myChangesCount);
+		ourLog.info(retval.summary());
 
 		if (isDryRun()) {
-			StringBuilder statementBuilder = buildExecutedStatementsString();
+			StringBuilder statementBuilder = buildExecutedStatementsString(retval);
 			ourLog.info("SQL that would be executed:\n\n***********************************\n{}***********************************", statementBuilder);
 		}
+
+		return retval;
+	}
+
+	private void preExecute(BaseTask theTask) {
+		myCallbacks.forEach(action -> action.preExecution(theTask));
+
 	}
 
 	private void postExecute(BaseTask theNext, StopWatch theStopWatch, boolean theSuccess) {
@@ -163,11 +167,11 @@ public class HapiMigrator {
 	}
 
 	@Nonnull
-	public List<Callback> getCallbacks() {
+	public List<IHapiMigrationCallback> getCallbacks() {
 		return myCallbacks;
 	}
 
-	public void setCallbacks(@Nonnull List<Callback> theCallbacks) {
+	public void setCallbacks(@Nonnull List<IHapiMigrationCallback> theCallbacks) {
 		Validate.notNull(theCallbacks);
 		myCallbacks = theCallbacks;
 	}

hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/IHapiMigrationCallback.java

@@ -0,0 +1,8 @@
+package ca.uhn.fhir.jpa.migrate;
+
+import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask;
+
+public interface IHapiMigrationCallback {
+	default void preExecution(BaseTask theTask) {}
+	default void postExecution(BaseTask theTask) {}
+}

hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/MigrationResult.java

@@ -0,0 +1,21 @@
+package ca.uhn.fhir.jpa.migrate;
+
+import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class MigrationResult {
+	public int changes = 0;
+	public final List<BaseTask.ExecutedStatement> executedStatements = new ArrayList<>();
+	public final List<BaseTask> succeededTasks = new ArrayList<>();
+	public final List<BaseTask> failedTasks = new ArrayList<>();
+
+	public String summary() {
+		return String.format("Completed executing %s migration tasks: %s succeeded, %s failed.  %s changes were applied to the database.",
+			succeededTasks.size() + failedTasks.size(),
+			succeededTasks.size(),
+			failedTasks.size(),
+			changes);
+	}
+}

hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/SchemaMigrator.java

@@ -24,7 +24,6 @@ import ca.uhn.fhir.context.ConfigurationException;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.jpa.migrate.taskdef.BaseTask;
 import org.flywaydb.core.api.MigrationVersion;
-import org.flywaydb.core.api.callback.Callback;
 import org.hibernate.cfg.AvailableSettings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -45,7 +44,7 @@ public class SchemaMigrator {
 	private final String myMigrationTableName;
 	private final List<BaseTask> myMigrationTasks;
 	private DriverTypeEnum myDriverType;
-	private List<Callback> myCallbacks = Collections.emptyList();
+	private List<IHapiMigrationCallback> myCallbacks = Collections.emptyList();
 	private final HapiMigrationStorageSvc myHapiMigrationStorageSvc;
 
 	/**
@@ -121,7 +120,7 @@ public class SchemaMigrator {
 		myDriverType = theDriverType;
 	}
 
-	public void setCallbacks(List<Callback> theCallbacks) {
+	public void setCallbacks(List<IHapiMigrationCallback> theCallbacks) {
 		myCallbacks = theCallbacks;
 	}
 

hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/InitializeSchemaTask.java

@@ -33,7 +33,7 @@ import java.util.List;
 import java.util.Set;
 
 public class InitializeSchemaTask extends BaseTask {
-	public static final String DESCRIPTION_PREFIX = "Initialize schema for ";
+	private static final String DESCRIPTION_PREFIX = "Initialize schema for ";
 	private static final Logger ourLog = LoggerFactory.getLogger(InitializeSchemaTask.class);
 	private final ISchemaInitializationProvider mySchemaInitializationProvider;
 	private boolean myInitializedSchema;

hapi-fhir-sql-migrate/src/test/java/ca/uhn/fhir/jpa/migrate/taskdef/DropTableTest.java

@@ -1,6 +1,7 @@
 package ca.uhn.fhir.jpa.migrate.taskdef;
 
 import ca.uhn.fhir.jpa.migrate.JdbcUtils;
+import ca.uhn.fhir.jpa.migrate.MigrationResult;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
@@ -10,6 +11,7 @@ import java.util.function.Supplier;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasItems;
 import static org.hamcrest.core.IsNot.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 public class DropTableTest extends BaseTest {
 
@@ -70,7 +72,7 @@ public class DropTableTest extends BaseTest {
 
 	@ParameterizedTest(name = "{index}: {0}")
 	@MethodSource("data")
-	public void testFlywayGetMigrationInfo(Supplier<TestDatabaseDetails> theTestDatabaseDetails) throws SQLException {
+	public void testHapiMigrationResult(Supplier<TestDatabaseDetails> theTestDatabaseDetails) throws SQLException {
 		before(theTestDatabaseDetails);
 
 		executeSql("create table SOMETABLE (PID bigint not null, TEXTCOL varchar(255))");
@@ -81,10 +83,11 @@ public class DropTableTest extends BaseTest {
 
 		assertThat(JdbcUtils.getTableNames(getConnectionProperties()), (hasItems("SOMETABLE")));
 
-// WIP KHS replace with dao
+		MigrationResult result = getMigrator().migrate();
-		//		assertThat(getMigrator().getMigrationInfo().get().pending().length, greaterThan(0));
+		assertEquals(0, result.changes);
-		getMigrator().migrate();
+		assertEquals(1, result.executedStatements.size());
-//		assertThat(getMigrator().getMigrationInfo().get().pending().length, equalTo(0));
+		assertEquals(1, result.succeededTasks.size());
+		assertEquals(0, result.failedTasks.size());
 
 		assertThat(JdbcUtils.getTableNames(getConnectionProperties()), not(hasItems("SOMETABLE")));
 	}