Refactor fastCheck, move startMs and move setAutoCommitFalse()

ebean-migration/src/main/java/io/ebean/migration/runner/FirstCheck.java

@@ -0,0 +1,89 @@
+package io.ebean.migration.runner;
+
+import io.ebean.migration.MigrationConfig;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+/**
+ * First initial check to see if migrations exist and exactly match.
+ */
+final class FirstCheck {
+
+  final MigrationConfig config;
+  final MigrationPlatform platform;
+  final Connection connection;
+  final String schema;
+  final String table;
+  final String sqlTable;
+  boolean tableKnownToExist;
+  private int count;
+
+  FirstCheck(MigrationConfig config, Connection connection, MigrationPlatform platform) {
+    this.config = config;
+    this.platform = platform;
+    this.connection = connection;
+    this.schema = config.getDbSchema();
+    this.table = config.getMetaTable();
+    this.sqlTable = schema != null ? schema + '.' + table : table;
+  }
+
+  MigrationTable initTable(boolean checkStateOnly) {
+    return new MigrationTable(this, checkStateOnly);
+  }
+
+  boolean fastModeCheck(List<LocalMigrationResource> versions) {
+    try {
+      final List<MigrationMetaRow> rows = fastRead();
+      tableKnownToExist = !rows.isEmpty();
+      if (rows.size() != versions.size() + 1) {
+        // difference in count of migrations
+        return false;
+      }
+      final Map<String, Integer> dbChecksums = dbChecksumMap(rows);
+      for (LocalMigrationResource local : versions) {
+        Integer dbChecksum = dbChecksums.get(local.key());
+        if (dbChecksum == null) {
+          // no match, unexpected missing migration
+          return false;
+        }
+        int localChecksum = checksumFor(local);
+        if (localChecksum != dbChecksum) {
+          // no match, perhaps repeatable migration change
+          return false;
+        }
+      }
+      // successful fast check
+      count = versions.size();
+      return true;
+    } catch (SQLException e) {
+      // probably migration table does not exist
+      return false;
+    }
+  }
+
+  private static Map<String, Integer> dbChecksumMap(List<MigrationMetaRow> rows) {
+    return rows.stream().collect(Collectors.toMap(MigrationMetaRow::version, MigrationMetaRow::checksum));
+  }
+
+  private int checksumFor(LocalMigrationResource local) {
+    if (local instanceof LocalUriMigrationResource) {
+      return ((LocalUriMigrationResource) local).checksum();
+    } else if (local instanceof LocalDdlMigrationResource) {
+      return Checksum.calculate(local.content());
+    } else {
+      return ((LocalJdbcMigrationResource) local).checksum();
+    }
+  }
+
+  List<MigrationMetaRow> fastRead() throws SQLException {
+    return platform.fastReadMigrations(sqlTable, connection);
+  }
+
+  int count() {
+    return count;
+  }
+}

ebean-migration/src/main/java/io/ebean/migration/runner/MigrationEngine.java

@@ -7,13 +7,11 @@
 
 import java.sql.Connection;
 import java.sql.SQLException;
-import java.util.Collections;
 import java.util.List;
-import java.util.Map;
-import java.util.stream.Collectors;
 
 import static java.lang.System.Logger.Level.*;
 import static java.lang.System.Logger.Level.WARNING;
+import static java.util.Collections.emptyList;
 
 /**
  * Actually runs the migrations.
@@ -25,8 +23,6 @@ public class MigrationEngine {
   private final MigrationConfig migrationConfig;
   private final boolean checkStateOnly;
   private final boolean fastMode;
-  private int fastModeCount;
-  private MigrationTable table;
 
   /**
    * Create with the MigrationConfig.
@@ -42,23 +38,25 @@ public MigrationEngine(MigrationConfig migrationConfig, boolean checkStateOnly)
    */
   public List<MigrationResource> run(Connection connection) {
     try {
+      long startMs = System.currentTimeMillis();
       LocalMigrationResources resources = new LocalMigrationResources(migrationConfig);
       if (!resources.readResources() && !resources.readInitResources()) {
         log.log(DEBUG, "no migrations to check");
-        return Collections.emptyList();
+        return emptyList();
       }
-      long startMs = System.currentTimeMillis();
-      setAutoCommitFalse(connection);
-      table = initMigrationTable(connection);
-      if (fastMode && fastModeCheck(resources.versions())) {
+      final var platform = derivePlatform(migrationConfig, connection);
+      final var firstCheck = new FirstCheck(migrationConfig, connection, platform);
+      if (fastMode && firstCheck.fastModeCheck(resources.versions())) {
         long checkMs = System.currentTimeMillis() - startMs;
-        log.log(INFO, "DB migrations completed in {0}ms - totalMigrations:{1}", checkMs, fastModeCount);
-        return Collections.emptyList();
+        log.log(INFO, "DB migrations completed in {0}ms - totalMigrations:{1}", checkMs, firstCheck.count());
+        return emptyList();
       }
+      // ensure running with autoCommit false
+      setAutoCommitFalse(connection);
 
-      initialiseMigrationTable(connection);
+      final MigrationTable table = initialiseMigrationTable(firstCheck, connection);
       try {
-        List<MigrationResource> result = runMigrations(resources.versions());
+        List<MigrationResource> result = runMigrations(table, resources.versions());
         connection.commit();
         if (!checkStateOnly) {
           long commitMs = System.currentTimeMillis();
@@ -92,57 +90,11 @@ private static void setAutoCommitFalse(Connection connection) {
     }
   }
 
-  private MigrationTable initMigrationTable(Connection connection) {
-    final MigrationPlatform platform = derivePlatformName(migrationConfig, connection);
-    return new MigrationTable(migrationConfig, connection, checkStateOnly, platform);
-  }
-
-  private boolean fastModeCheck(List<LocalMigrationResource> versions) {
-    try {
-      final List<MigrationMetaRow> rows = table.fastRead();
-      if (rows.size() != versions.size() + 1) {
-        // difference in count of migrations
-        return false;
-      }
-      final Map<String, Integer> dbChecksums = dbChecksumMap(rows);
-      for (LocalMigrationResource local : versions) {
-        Integer dbChecksum = dbChecksums.get(local.key());
-        if (dbChecksum == null) {
-          // no match, unexpected missing migration
-          return false;
-        }
-        int localChecksum = checksumFor(local);
-        if (localChecksum != dbChecksum) {
-          // no match, perhaps repeatable migration change
-          return false;
-        }
-      }
-      // successful fast check
-      fastModeCount = versions.size();
-      return true;
-    } catch (SQLException e) {
-      // probably migration table does not exist
-      return false;
-    }
-  }
-
-  private static Map<String, Integer> dbChecksumMap(List<MigrationMetaRow> rows) {
-    return rows.stream().collect(Collectors.toMap(MigrationMetaRow::version, MigrationMetaRow::checksum));
-  }
-
-  private int checksumFor(LocalMigrationResource local) {
-    if (local instanceof LocalUriMigrationResource) {
-      return ((LocalUriMigrationResource)local).checksum();
-    } else if (local instanceof LocalDdlMigrationResource) {
-      return Checksum.calculate(local.content());
-    } else {
-      return ((LocalJdbcMigrationResource) local).checksum();
-    }
-  }
-
-  private void initialiseMigrationTable(Connection connection) {
+  private MigrationTable initialiseMigrationTable(FirstCheck firstCheck, Connection connection) {
     try {
+      final MigrationTable table = firstCheck.initTable(checkStateOnly);
       table.createIfNeededAndLock();
+      return table;
     } catch (Throwable e) {
       rollback(connection);
       throw new MigrationException("Error initialising db migrations table", e);
@@ -152,7 +104,7 @@ private void initialiseMigrationTable(Connection connection) {
   /**
    * Run all the migrations as needed.
    */
-  private List<MigrationResource> runMigrations(List<LocalMigrationResource> localVersions) throws SQLException {
+  private List<MigrationResource> runMigrations(MigrationTable table, List<LocalMigrationResource> localVersions) throws SQLException {
     // get the migrations in version order
     if (table.isEmpty()) {
       LocalMigrationResource initVersion = lastInitVersion();
@@ -182,7 +134,7 @@ private LocalMigrationResource lastInitVersion() {
   /**
    * Return the platform deriving from connection if required.
    */
-  private MigrationPlatform derivePlatformName(MigrationConfig migrationConfig, Connection connection) {
+  private MigrationPlatform derivePlatform(MigrationConfig migrationConfig, Connection connection) {
     final String platform = migrationConfig.getPlatform();
     if (platform != null) {
       return DbNameUtil.platform(platform);

ebean-migration/src/main/java/io/ebean/migration/runner/MigrationTable.java

@@ -74,13 +74,15 @@ final class MigrationTable {
   private MigrationMetaRow initMetaRow;
   private boolean tableKnownToExist;
 
-  /**
-   * Construct with server, configuration and jdbc connection (DB admin user).
-   */
-  MigrationTable(MigrationConfig config, Connection connection, boolean checkStateOnly, MigrationPlatform platform) {
-    this.config = config;
-    this.platform = platform;
-    this.connection = connection;
+  public MigrationTable(FirstCheck firstCheck, boolean checkStateOnly) {
+    this.config = firstCheck.config;
+    this.platform = firstCheck.platform;
+    this.connection = firstCheck.connection;
+    this.schema = firstCheck.schema;
+    this.table = firstCheck.table;
+    this.sqlTable = firstCheck.sqlTable;
+    this.tableKnownToExist = firstCheck.tableKnownToExist;
+
     this.scriptRunner = new MigrationScriptRunner(connection, platform);
     this.checkStateOnly = checkStateOnly;
     this.earlyChecksumMode = config.isEarlyChecksumMode();
@@ -93,11 +95,8 @@ final class MigrationTable {
     this.minVersionFailMessage = config.getMinVersionFailMessage();
     this.skipMigrationRun = config.isSkipMigrationRun();
     this.skipChecksum = config.isSkipChecksum();
-    this.schema = config.getDbSchema();
-    this.table = config.getMetaTable();
     this.basePlatformName = config.getBasePlatform();
     this.platformName = config.getPlatform();
-    this.sqlTable = initSqlTable();
     this.insertSql = MigrationMetaRow.insertSql(sqlTable);
     this.updateSql = MigrationMetaRow.updateSql(sqlTable);
     this.updateChecksumSql = MigrationMetaRow.updateChecksumSql(sqlTable);
@@ -109,14 +108,6 @@ private MigrationVersion initMinVersion(String minVersion) {
     return (minVersion == null || minVersion.isEmpty()) ? null : MigrationVersion.parse(minVersion);
   }
 
-  private String initSqlTable() {
-    if (schema != null) {
-      return schema + "." + table;
-    } else {
-      return table;
-    }
-  }
-
   private String sqlPrimaryKey() {
     return "pk_" + table;
   }
@@ -193,13 +184,6 @@ void unlockMigrationTable() {
     platform.unlockMigrationTable(sqlTable, connection);
   }
 
-  List<MigrationMetaRow> fastRead() throws SQLException {
-    final var result = platform.fastReadMigrations(sqlTable, connection);
-    // if we know the migration table exists we can skip those checks
-    tableKnownToExist = !result.isEmpty();
-    return result;
-  }
-
   /**
    * Read the migration table with details on what migrations have run.
    * This must execute after we have completed the wait for the lock on

ebean-migration/src/test/java/io/ebean/migration/runner/MigrationTable1Test.java

@@ -42,6 +42,12 @@ public void shutdown() {
     dataSource.shutdown();
   }
 
+
+  private MigrationTable migrationTable(Connection conn) {
+    var fc = new FirstCheck(config, conn, platform);
+    return new MigrationTable(fc, false);
+  }
+
   @Test
   public void testMigrationTableBase() throws Exception {
 
@@ -50,7 +56,7 @@ public void testMigrationTableBase() throws Exception {
     MigrationRunner runner = new MigrationRunner(config);
     runner.run(dataSource);
     try (Connection conn = dataSource.getConnection()) {
-      MigrationTable table = new MigrationTable(config, conn, false, platform);
+      MigrationTable table = migrationTable(conn);
       table.createIfNeededAndLock();
       assertThat(table.versions()).containsExactly("hello", "1.1", "1.2", "1.2.1", "m2_view");
 
@@ -68,6 +74,7 @@ public void testMigrationTableBase() throws Exception {
     }
   }
 
+
   @Test
   public void testMigrationTableRepeatableOk() throws Exception {
 
@@ -77,7 +84,7 @@ public void testMigrationTableRepeatableOk() throws Exception {
     runner.run(dataSource);
 
     try (Connection conn = dataSource.getConnection()) {
-      MigrationTable table = new MigrationTable(config, conn, false, platform);
+      MigrationTable table = migrationTable(conn);
       table.createIfNeededAndLock();
       assertThat(table.versions()).containsExactly("1.1");
       table.unlockMigrationTable();
@@ -90,7 +97,7 @@ public void testMigrationTableRepeatableOk() throws Exception {
     runner.run(dataSource);
 
     try (Connection conn = dataSource.getConnection()) {
-      MigrationTable table = new MigrationTable(config, conn, false, platform);
+      MigrationTable table = migrationTable(conn);
       table.createIfNeededAndLock();
       assertThat(table.versions()).containsExactly("1.1", "1.2", "m2_view");
       table.unlockMigrationTable();
@@ -108,7 +115,7 @@ public void testMigrationTableRepeatableFail() throws Exception {
     runner.run(dataSource);
 
     try (Connection conn = dataSource.getConnection()) {
-      MigrationTable table = new MigrationTable(config, conn, false, platform);
+      MigrationTable table = migrationTable(conn);
       table.createIfNeededAndLock();
       assertThat(table.versions()).containsExactly("1.1");
       table.unlockMigrationTable();
@@ -141,7 +148,7 @@ public void testMigrationTableRepeatableFail() throws Exception {
       assertThat(m3Content).contains("text with ; sign");
 
       // we expect, that 1.1 and 1.2 is executed (but not the R__ script)
-      MigrationTable table = new MigrationTable(config, conn, false, platform);
+      MigrationTable table = migrationTable(conn);
       table.createIfNeededAndLock();
       assertThat(table.versions()).containsExactly("1.1", "1.2");
       conn.rollback();