review: minor refactorings, improved log messages and an additional unit test

SVN: 760

dbmigration/source/java/ch/systemsx/cisd/dbmigration/DBMigrationEngine.java

@@ -81,18 +81,18 @@ public class DBMigrationEngine
         String databaseVersion = entry.getVersion();
         if (version.equals(databaseVersion))
         {
-            if (operationLog.isInfoEnabled())
+            if (operationLog.isDebugEnabled())
             {
                 String databaseName = adminDAO.getDatabaseName();
-                operationLog.info("No migration needed for database '" + databaseName + "'. Current version: " 
-                                  + version + ".");
+                operationLog.debug("No migration needed for database '" + databaseName + "'. It has the right version (" 
+                                  + version + ").");
             }
         } else if (version.compareTo(databaseVersion) > 0)
         {
             if (operationLog.isInfoEnabled())
             {
                 String databaseName = adminDAO.getDatabaseName();
-                operationLog.info("Migrating database '" + databaseName + "' from version " + databaseVersion 
+                operationLog.info("Trying to migrate database '" + databaseName + "' from version " + databaseVersion 
                                    + " to " + version + ".");
             }
             migrate(databaseVersion, version);
@@ -105,8 +105,8 @@ public class DBMigrationEngine
         } else
         {
             String databaseName = adminDAO.getDatabaseName();
-            String message = "Couldn't revert database '" + databaseName + "' from version " 
-                             + databaseVersion + " to previous version " + version + ".";
+            String message = "Cannot revert database '" + databaseName + "' from version " 
+                             + databaseVersion + " to earlier version " + version + ".";
             operationLog.error(message);
             throw new EnvironmentFailureException(message);
         }
@@ -190,8 +190,8 @@ public class DBMigrationEngine
             Script migrationScript = scriptProvider.getMigrationScript(version, nextVersion);
             if (migrationScript == null)
             {
-                String databaseName = adminDAO.getDatabaseName();
-                String message = "Cannot migrate database '" + databaseName + "' from version " + version + " to " 
+                final String databaseName = adminDAO.getDatabaseName();
+                final String message = "Cannot migrate database '" + databaseName + "' from version " + version + " to " 
                                  + nextVersion + " because of missing migration script.";
                 operationLog.error(message);
                 throw new EnvironmentFailureException(message);
@@ -201,7 +201,8 @@ public class DBMigrationEngine
         } while (version.equals(toVersion) == false);
     }
     
-    private String increment(String version)
+    // @Private
+    static String increment(String version)
     {
         char[] characters = new char[version.length()];
         version.getChars(0, characters.length, characters, 0);
@@ -220,10 +221,10 @@ public class DBMigrationEngine
         return new String(characters);
     }
 
-    private void executeScript(Script script, String version) throws Error
+    private void executeScript(Script script, String version)
     {
-        String code = script.getCode();
-        String name = script.getName();
+        final String name = script.getName();
+        final String code = script.getCode();
         logDAO.logStart(version, name, code);
         try
         {
@@ -231,7 +232,7 @@ public class DBMigrationEngine
             logDAO.logSuccess(version, name);
         } catch (Throwable t)
         {
-            operationLog.error("Executing script '" + name + "' failed", t);
+            operationLog.error("Executing script '" + name + "' failed.", t);
             logDAO.logFailure(version, name, t);
             if (t instanceof RuntimeException)
             {

dbmigration/source/java/ch/systemsx/cisd/dbmigration/DBUtilities.java

@@ -40,4 +40,15 @@ class DBUtilities
     }
 
     private DBUtilities() {}
+
+    /**
+     * Checks whether given <code>DataAccessException</code> is caused by a "user already exists" exception.
+     * <p>
+     * This is database specific.
+     * </p>
+     */
+    static boolean ownerAlreadyExists(DataAccessException ex) {
+        // 42710 DUPLICATE OBJECT
+        return SQLStateUtils.isDuplicateObject(SQLStateUtils.getSqlState(ex));
+    }
 }

dbmigration/source/java/ch/systemsx/cisd/dbmigration/PostgreSQLAdminDAO.java

@@ -20,28 +20,27 @@ import javax.sql.DataSource;
 
 import org.apache.log4j.Logger;
 import org.springframework.dao.DataAccessException;
-import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.core.simple.SimpleJdbcDaoSupport;
 
-import ch.systemsx.cisd.common.db.SQLStateUtils;
 import ch.systemsx.cisd.common.logging.LogCategory;
 import ch.systemsx.cisd.common.logging.LogFactory;
 
 /**
  * Implementation of {@link IDatabaseAdminDAO} for PostgreSQL.
- *
+ * 
  * @author Franz-Josef Elmer
  */
 public class PostgreSQLAdminDAO extends SimpleJdbcDaoSupport implements IDatabaseAdminDAO
 {
     private static final Logger operationLog = LogFactory.getLogger(LogCategory.OPERATION, PostgreSQLAdminDAO.class);
-    
+
     private final String owner;
+
     private final String database;
 
     /**
      * Creates an instance.
-     *
+     * 
      * @param dataSource Data source able to create/drop the specified database.
      * @param owner Owner to be created if it doesn't exist.
      * @param database Name of the database.
@@ -52,7 +51,7 @@ public class PostgreSQLAdminDAO extends SimpleJdbcDaoSupport implements IDatabas
         this.database = database;
         setDataSource(dataSource);
     }
-    
+
     public String getDatabaseName()
     {
         return database;
@@ -65,38 +64,28 @@ public class PostgreSQLAdminDAO extends SimpleJdbcDaoSupport implements IDatabas
             getJdbcTemplate().execute("create user " + owner);
         } catch (DataAccessException ex)
         {
-            if (ownerAlreadyExists(ex))
+            if (DBUtilities.ownerAlreadyExists(ex))
             {
                 if (operationLog.isInfoEnabled())
                 {
                     operationLog.info("Owner '" + owner + "' already exists.");
                 }
-            } else {
+            } else
+            {
                 operationLog.error("Database owner couldn't be created:", ex);
                 throw ex;
             }
         }
     }
 
-    /**
-     * Checks whether given <code>DataAccessException</code> is caused by a "user already exists" exception.
-     * <p>
-     * This is database specific.
-     * </p>
-     */
-    protected boolean ownerAlreadyExists(DataAccessException ex) {
-        // 42710 DUPLICATE OBJECT
-        return SQLStateUtils.isDuplicateObject(SQLStateUtils.getSqlState(ex));
-    }
-    
     public void createDatabase()
     {
-        JdbcTemplate template = getJdbcTemplate();
-        template.execute("create database " + database + " with owner = " + owner 
-                         + " encoding = 'utf8' tablespace = pg_default;"
-                         + "alter database " + database + " set default_with_oids = off;");
+        getJdbcTemplate().execute(
+                "create database " + database + " with owner = " + owner
+                        + " encoding = 'utf8' tablespace = pg_default;" + "alter database " + database
+                        + " set default_with_oids = off;");
     }
-    
+
     public void dropDatabase()
     {
         try
@@ -110,5 +99,5 @@ public class PostgreSQLAdminDAO extends SimpleJdbcDaoSupport implements IDatabas
             }
         }
     }
-    
+
 }

dbmigration/source/java/ch/systemsx/cisd/dbmigration/PostgreSQLDAOFactory.java

@@ -35,7 +35,7 @@ public class PostgreSQLDAOFactory implements IDAOFactory
     public PostgreSQLDAOFactory(DatabaseConfigurationContext context)
     {
         databaseDAO = new PostgreSQLAdminDAO(context.getAdminDataSource(), context.getOwner(), context.getDatabaseName());
-        DataSource dataSource = context.getDataSource();
+        final DataSource dataSource = context.getDataSource();
         sqlScriptExecutor = new SqlScriptExecutor(dataSource);
         databaseVersionLogDAO = new DatabaseVersionLogDAO(dataSource, context.getLobHandler());
     }