Improve logging message, remove javax NonNull annotation

rob-bygrave · rob-bygrave · commit e4b01e3c0fec · 2023-05-31T08:12:12.000+12:00
Should migrate to using NonNullApi instead
diff --git a/ebean-migration/pom.xml b/ebean-migration/pom.xml
@@ -47,13 +47,6 @@
       <version>7.1</version>
     </dependency>
 
-    <dependency>
-      <groupId>io.avaje</groupId>
-      <artifactId>avaje-jsr305</artifactId>
-      <version>1.2</version>
-      <scope>provided</scope>
-    </dependency>
-
     <!-- test dependencies -->
 
     <dependency>
diff --git a/ebean-migration/src/main/java/io/ebean/migration/DbNameUtil.java b/ebean-migration/src/main/java/io/ebean/migration/DbNameUtil.java
@@ -2,7 +2,6 @@
 
 import io.ebean.migration.runner.MigrationPlatform;
 
-import javax.annotation.Nonnull;
 import java.sql.*;
 
 import static java.lang.System.Logger.Level.WARNING;
@@ -17,7 +16,6 @@ class DbNameUtil implements DbPlatformNames {
    * <p>
    * At this point only sql server has platform specific handling required (create table and for update).
    */
-  @Nonnull
   static String normalise(Connection connection) {
     try {
       final String productName = connection.getMetaData().getDatabaseProductName().toLowerCase();
@@ -68,7 +66,6 @@ private static String readPostgres(Connection connection) {
     return POSTGRES;
   }
 
-  @Nonnull
   static MigrationPlatform platform(String platformName) {
     switch (platformName) {
       case MYSQL:
diff --git a/ebean-migration/src/main/java/io/ebean/migration/JdbcMigrationFactory.java b/ebean-migration/src/main/java/io/ebean/migration/JdbcMigrationFactory.java
@@ -1,7 +1,5 @@
 package io.ebean.migration;
 
-import javax.annotation.Nonnull;
-
 /**
  * Factory to create and initialise a JdbcMigration.
  *
@@ -12,6 +10,5 @@ public interface JdbcMigrationFactory {
   /**
    * Create a JDBC based migration given the class name.
    */
-  @Nonnull
   JdbcMigration createInstance(String className);
 }
diff --git a/ebean-migration/src/main/java/io/ebean/migration/MigrationConfig.java b/ebean-migration/src/main/java/io/ebean/migration/MigrationConfig.java
@@ -1,6 +1,5 @@
 package io.ebean.migration;
 
-import javax.annotation.Nonnull;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.SQLException;
@@ -583,7 +582,6 @@ public void setPlatform(String platform) {
    */
   public class DefaultMigrationFactory implements JdbcMigrationFactory {
 
-    @Nonnull
     @Override
     public JdbcMigration createInstance(String className) {
       try {
diff --git a/ebean-migration/src/main/java/io/ebean/migration/MigrationRunner.java b/ebean-migration/src/main/java/io/ebean/migration/MigrationRunner.java
@@ -3,7 +3,6 @@
 import io.avaje.applog.AppLog;
 import io.ebean.migration.runner.*;
 
-import javax.annotation.Nonnull;
 import javax.sql.DataSource;
 import java.sql.Connection;
 import java.sql.SQLException;
@@ -29,23 +28,20 @@ public MigrationRunner(MigrationConfig migrationConfig) {
   /**
    * Return the migrations that would be applied if the migration is run.
    */
-  @Nonnull
   public List<LocalMigrationResource> checkState() {
     return checkState(migrationConfig.createConnection());
   }
 
   /**
    * Return the migrations that would be applied if the migration is run.
    */
-  @Nonnull
   public List<LocalMigrationResource> checkState(DataSource dataSource) {
     return checkState(getConnection(dataSource));
   }
 
   /**
    * Return the migrations that would be applied if the migration is run.
    */
-  @Nonnull
   public List<LocalMigrationResource> checkState(Connection connection) {
     run(connection, true);
     return checkMigrations;
@@ -97,15 +93,21 @@ protected void run(Connection connection, boolean checkStateMode) {
         return;
       }
 
+      long start = System.currentTimeMillis();
       connection.setAutoCommit(false);
       MigrationPlatform platform = derivePlatformName(migrationConfig, connection);
       new MigrationSchema(migrationConfig, connection).createAndSetIfNeeded();
 
       MigrationTable table = new MigrationTable(migrationConfig, connection, checkStateMode, platform);
       table.createIfNeededAndLock();
       try {
-        runMigrations(resources, table, checkStateMode);
+        List<LocalMigrationResource> migrations = resources.getVersions();
+        runMigrations(migrations, table, checkStateMode);
         connection.commit();
+        if (!checkStateMode) {
+          long exeMillis = System.currentTimeMillis() - start;
+          log.log(INFO, "DB migrations completed in {0}ms - executed:{1} size:{2}", exeMillis, table.count(), migrations.size());
+        }
         table.runNonTransactional();
       } finally {
         table.unlockMigrationTable();
@@ -127,9 +129,8 @@ protected void run(Connection connection, boolean checkStateMode) {
   /**
    * Run all the migrations as needed.
    */
-  private void runMigrations(LocalMigrationResources resources, MigrationTable table, boolean checkStateMode) throws SQLException {
+  private void runMigrations(List<LocalMigrationResource> localVersions, MigrationTable table, boolean checkStateMode) throws SQLException {
     // get the migrations in version order
-    List<LocalMigrationResource> localVersions = resources.getVersions();
     if (table.isEmpty()) {
       LocalMigrationResource initVersion = getInitVersion();
       if (initVersion != null) {
@@ -139,7 +140,6 @@ private void runMigrations(LocalMigrationResources resources, MigrationTable tab
         return;
       }
     }
-    log.log(INFO, "Local migrations:{0}  existing migrations:{1}  checkState:{2}", localVersions.size(), table.size(), checkStateMode);
     checkMigrations = table.runAll(localVersions);
   }
 
diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/LocalDdlMigrationResource.java b/ebean-migration/src/main/java/io/ebean/migration/runner/LocalDdlMigrationResource.java
@@ -3,7 +3,6 @@
 import io.avaje.classpath.scanner.Resource;
 import io.ebean.migration.MigrationVersion;
 
-import javax.annotation.Nonnull;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 
@@ -25,7 +24,6 @@ public LocalDdlMigrationResource(MigrationVersion version, String location, Reso
   /**
    * Return the content for the migration apply ddl script.
    */
-  @Nonnull
   public String getContent() {
     try {
       return resource.loadAsString(StandardCharsets.UTF_8);
diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/LocalJdbcMigrationResource.java b/ebean-migration/src/main/java/io/ebean/migration/runner/LocalJdbcMigrationResource.java
@@ -4,8 +4,6 @@
 import io.ebean.migration.MigrationChecksumProvider;
 import io.ebean.migration.MigrationVersion;
 
-import javax.annotation.Nonnull;
-
 /**
  * A DB migration resource (JdbcMigration with version).
  *
@@ -26,7 +24,6 @@ public LocalJdbcMigrationResource(MigrationVersion version, String location, Jdb
   /**
    * Return the migration
    */
-  @Nonnull
   public JdbcMigration getMigration() {
     return migration;
   }
@@ -42,7 +39,6 @@ public int getChecksum() {
     }
   }
 
-  @Nonnull
   @Override
   public String getContent() {
     return "location:" + location;
diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/LocalMigrationResource.java b/ebean-migration/src/main/java/io/ebean/migration/runner/LocalMigrationResource.java
@@ -2,8 +2,6 @@
 
 import io.ebean.migration.MigrationVersion;
 
-import javax.annotation.Nonnull;
-
 /**
  * A DB migration resource (DDL or Jdbc)
  */
@@ -52,7 +50,6 @@ public boolean isRepeatableLast() {
   /**
    * Return the "key" that identifies the migration.
    */
-  @Nonnull
   public String key() {
     if (isRepeatable()) {
       return version.getComment().toLowerCase();
@@ -64,7 +61,6 @@ public String key() {
   /**
    * Return the migration comment.
    */
-  @Nonnull
   public String getComment() {
     String comment = version.getComment();
     return (comment == null || comment.isEmpty()) ? "-" : comment;
@@ -81,7 +77,6 @@ public int compareTo(LocalMigrationResource o) {
   /**
    * Return the underlying migration version.
    */
-  @Nonnull
   public MigrationVersion getVersion() {
     return version;
   }
@@ -96,13 +91,11 @@ public String getLocation() {
   /**
    * Return the content of the migration.
    */
-  @Nonnull
   public abstract String getContent();
 
   /**
    * Return the type code ("R" or "V") for this migration.
    */
-  @Nonnull
   public String getType() {
     return type;
   }
diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/LocalMigrationResources.java b/ebean-migration/src/main/java/io/ebean/migration/runner/LocalMigrationResources.java
@@ -6,7 +6,6 @@
 import io.ebean.migration.MigrationConfig;
 import io.ebean.migration.MigrationVersion;
 
-import javax.annotation.Nonnull;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -135,7 +134,6 @@ private LocalMigrationResource createScriptMigration(Resource resource, String f
   /**
    * Return the list of migration resources in version order.
    */
-  @Nonnull
   public List<LocalMigrationResource> getVersions() {
     return versions;
   }
diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationPlatform.java b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationPlatform.java
@@ -2,7 +2,6 @@
 
 import io.ebean.ddlrunner.DdlDetect;
 
-import javax.annotation.Nonnull;
 import java.sql.*;
 import java.util.ArrayList;
 import java.util.List;
@@ -80,7 +79,6 @@ private int lockRows(String sqlTable, Connection connection) throws SQLException
   /**
    * Read the existing migrations from the db migration table.
    */
-  @Nonnull
   List<MigrationMetaRow> readExistingMigrations(String sqlTable, Connection connection) throws SQLException {
     final String selectSql = sqlSelectForReading(sqlTable);
     List<MigrationMetaRow> rows = new ArrayList<>();
@@ -97,15 +95,13 @@ List<MigrationMetaRow> readExistingMigrations(String sqlTable, Connection connec
   /**
    * Return the SQL to lock the rows in db migration table with row locking.
    */
-  @Nonnull
   String sqlSelectForUpdate(String table) {
     return BASE_SELECT_ID + table + forUpdateSuffix;
   }
 
   /**
    * Return the SQL to read the db migration table.
    */
-  @Nonnull
   String sqlSelectForReading(String table) {
     return BASE_SELECT_ALL + table + forUpdateSuffix;
   }
diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationSchema.java b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationSchema.java
@@ -8,6 +8,7 @@
 import java.sql.SQLException;
 import java.sql.Statement;
 
+import static java.lang.System.Logger.Level.DEBUG;
 import static java.lang.System.Logger.Level.INFO;
 
 /**
@@ -41,7 +42,7 @@ private String trim(String dbSchema) {
    */
   public void createAndSetIfNeeded() throws SQLException {
     if (dbSchema != null) {
-      log.log(INFO, "Migration Schema: {0}", dbSchema);
+      log.log(DEBUG, "Migration using schema: {0}", dbSchema);
       if (createSchemaIfNotExists) {
         createSchemaIfNeeded();
       }
@@ -53,7 +54,7 @@ public void createAndSetIfNeeded() throws SQLException {
 
   private void createSchemaIfNeeded() throws SQLException {
     if (!schemaExists()) {
-      log.log(INFO, "Creating Schema: {0}", dbSchema);
+      log.log(INFO, "Creating schema: {0}", dbSchema);
       try (Statement query = connection.createStatement()) {
         query.executeUpdate("CREATE SCHEMA " + dbSchema);
       }
@@ -73,7 +74,7 @@ private boolean schemaExists() throws SQLException {
   }
 
   private void setSchema() throws SQLException {
-    log.log(INFO, "Setting Schema: {0}", dbSchema);
+    log.log(DEBUG, "Setting schema: {0}", dbSchema);
     connection.setSchema(dbSchema);
   }
 
diff --git a/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationTable.java b/ebean-migration/src/main/java/io/ebean/migration/runner/MigrationTable.java
@@ -7,7 +7,6 @@
 import io.ebean.migration.MigrationException;
 import io.ebean.migration.MigrationVersion;
 
-import javax.annotation.Nonnull;
 import java.io.IOException;
 import java.net.URL;
 import java.sql.*;
@@ -68,6 +67,8 @@ public final class MigrationTable {
    */
   private MigrationVersion dbInitVersion;
 
+  private int executionCount;
+
   /**
    * Construct with server, configuration and jdbc connection (DB admin user).
    */
@@ -123,7 +124,6 @@ public int size() {
   /**
    * Returns the versions that are already applied.
    */
-  @Nonnull
   public Set<String> getVersions() {
     return migrations.keySet();
   }
@@ -209,7 +209,6 @@ void createTable() throws IOException, SQLException {
   /**
    * Return the create table script.
    */
-  @Nonnull
   String createTableDdl() throws IOException {
     String script = ScriptTransform.replace("${table}", sqlTable, getCreateTableScript());
     return ScriptTransform.replace("${pk_table}", sqlPrimaryKey(), script);
@@ -414,6 +413,7 @@ private long executeMigration(LocalMigrationResource local, String script) throw
       log.log(INFO, "Executing jdbc migration version: {0} - {1}", local.getVersion(), migration);
       migration.migrate(connection);
     }
+    executionCount++;
     return System.currentTimeMillis() - start;
   }
 
@@ -493,11 +493,9 @@ public boolean isEmpty() {
   /**
    * Run all the migrations in order as needed.
    *
-   * @return the migrations that have been run (collected if checkstate is true).
+   * @return the migrations that have been run (collected if checkState is true).
    */
-  @Nonnull
   public List<LocalMigrationResource> runAll(List<LocalMigrationResource> localVersions) throws SQLException {
-
     checkMinVersion();
     for (LocalMigrationResource localVersion : localVersions) {
       if (!localVersion.isRepeatable() && dbInitVersion != null && dbInitVersion.compareTo(localVersion.getVersion()) >= 0) {
@@ -525,11 +523,8 @@ private void checkMinVersion() {
    *
    * @return the migrations that have been run (collected if checkstate is true).
    */
-  @Nonnull
   public List<LocalMigrationResource> runInit(LocalMigrationResource initVersion, List<LocalMigrationResource> localVersions) throws SQLException {
-
     runRepeatableInit(localVersions);
-
     initVersion.setInitType();
     if (!shouldRun(initVersion, null)) {
       throw new IllegalStateException("Expected to run init migration but it didn't?");
@@ -562,4 +557,8 @@ private void runRepeatableInit(List<LocalMigrationResource> localVersions) throw
   public void runNonTransactional() {
     scriptRunner.runNonTransactional();
   }
+
+  public int count() {
+    return executionCount;
+  }
 }
diff --git a/ebean-migration/src/main/java/module-info.java b/ebean-migration/src/main/java/module-info.java
@@ -8,7 +8,6 @@
   requires transitive io.avaje.classpath.scanner;
   requires transitive io.ebean.ddl.runner;
   requires io.ebean.migration.auto;
-  requires static io.avaje.jsr305x;
 
   provides io.ebean.migration.auto.AutoMigrationRunner with io.ebean.migration.AutoRunner;
 }

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,5 @@`
`1`	`1`	`package io.ebean.migration;`
`2`	`2`
`3`		`-import javax.annotation.Nonnull;`
`4`		`-`
`5`	`3`	`/**`
`6`	`4`	`* Factory to create and initialise a JdbcMigration.`
`7`	`5`	`*`
`@@ -12,6 +10,5 @@ public interface JdbcMigrationFactory {`
`12`	`10`	`/**`
`13`	`11`	`* Create a JDBC based migration given the class name.`
`14`	`12`	`*/`
`15`		`- @Nonnull`
`16`	`13`	`JdbcMigration createInstance(String className);`
`17`	`14`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,8 +4,6 @@`
`4`	`4`	`import io.ebean.migration.MigrationChecksumProvider;`
`5`	`5`	`import io.ebean.migration.MigrationVersion;`
`6`	`6`
`7`		`-import javax.annotation.Nonnull;`
`8`		`-`
`9`	`7`	`/**`
`10`	`8`	`* A DB migration resource (JdbcMigration with version).`
`11`	`9`	`*`
`@@ -26,7 +24,6 @@ public LocalJdbcMigrationResource(MigrationVersion version, String location, Jdb`
`26`	`24`	`/**`
`27`	`25`	`* Return the migration`
`28`	`26`	`*/`
`29`		`- @Nonnull`
`30`	`27`	`public JdbcMigration getMigration() {`
`31`	`28`	`return migration;`
`32`	`29`	`}`
`@@ -42,7 +39,6 @@ public int getChecksum() {`
`42`	`39`	`}`
`43`	`40`	`}`
`44`	`41`
`45`		`- @Nonnull`
`46`	`42`	`@Override`
`47`	`43`	`public String getContent() {`
`48`	`44`	`return "location:" + location;`