Refactor JsonDatabaseMigration to make easier to implement future JSON setting migrations

core/src/main/java/org/fao/geonet/migration/JsonDatabaseMigration.java

@@ -27,8 +27,15 @@
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.lang3.StringUtils;
+import org.fao.geonet.DatabaseMigrationTask;
+import org.fao.geonet.constants.Geonet;
+import org.fao.geonet.utils.Log;
 
 import java.io.IOException;
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
@@ -38,7 +45,36 @@
 /**
  * Helper class for handling JSON strings.
  */
-public abstract class JsonDatabaseMigration {
+public abstract class JsonDatabaseMigration implements DatabaseMigrationTask {
+
+    @Override
+    public void update(Connection connection) throws SQLException {
+        try (PreparedStatement statement = connection.prepareStatement(
+            "SELECT * FROM settings WHERE name=?")) {
+            statement.setString(1, getSettingName());
+            ResultSet rs = statement.executeQuery();
+
+            if (rs.next()) {
+                String currentSettingJson = rs.getString("value");
+                Map<String, String> fieldsToUpdate = setUpNewSettingValues();
+
+                String newSettingJson = insertOrUpdateField(currentSettingJson, fieldsToUpdate);
+                try (PreparedStatement updateStatement = connection.prepareStatement("UPDATE settings SET "
+                    + "value=? WHERE name=?")) {
+                    updateStatement.setString(1, newSettingJson);
+                    updateStatement.setString(2, getSettingName());
+                    updateStatement.executeUpdate();
+                }
+            }
+        } catch (IOException e) {
+            Log.error(Geonet.GEONETWORK + ".databasemigration",
+                "Error in AdvancedSearchFormMigration. Cannot complete the "
+                    + "migration", e);
+            throw new DatabaseMigrationException(e);
+        }
+    }
+
+
     /**
      * Given a JSON string and a map of fields to update/insert and their values return an updated JSON string.
      *
@@ -80,4 +116,18 @@ protected void createMissingNodes(JsonNode root, List<String> fullPath, JsonNode
         }
         ((ObjectNode) newRoot).set(fullPath.get(fullPath.size() - 1), newJsonTree);
     }
+
+
+    /**
+     *
+     * @return a map where the key is the JSON path to the property to add or modify and the values are the new values
+     * for the path in key.
+     */
+    protected abstract Map<String, String> setUpNewSettingValues();
+
+    /**
+     *
+     * @return the name of the setting to update.
+     */
+    protected abstract String getSettingName();
 }

web/src/main/java/org/fao/geonet/DatabaseMigration.java

@@ -24,11 +24,8 @@
 package org.fao.geonet;
 
 import com.google.common.util.concurrent.Callables;
-
 import com.vividsolutions.jts.util.Assert;
-
 import jeeves.server.sources.http.ServletPathFinder;
-
 import org.fao.geonet.constants.Geonet;
 import org.fao.geonet.domain.Pair;
 import org.fao.geonet.kernel.setting.Settings;
@@ -42,6 +39,8 @@
 import org.springframework.orm.jpa.JpaTransactionManager;
 import org.springframework.web.context.WebApplicationContext;
 
+import javax.servlet.ServletContext;
+import javax.sql.DataSource;
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.io.UnsupportedEncodingException;
@@ -50,14 +49,10 @@
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
 
-import javax.servlet.ServletContext;
-import javax.sql.DataSource;
-
 /**
  * Postprocessor that runs after the jdbcDataSource bean has been initialized and migrates the
  * database as soon as it is.
@@ -74,7 +69,7 @@ public class DatabaseMigration implements BeanPostProcessor {
     @Autowired
     private ApplicationContext _applicationContext;
 
-    private Callable<LinkedHashMap<String, List<String>>> _migration;
+    private Callable<Map<String, List<String>>> _migration;
 
     private String initAfter;
 
@@ -134,35 +129,27 @@ public final Object postProcessAfterInitialization(final Object bean, final Stri
         return bean;
     }
 
-    public final void setMigration(final LinkedHashMap<String, List<String>> migration) {
+    public final void setMigration(final Map<String, List<String>> migration) {
         this._migration = Callables.returning(migration);
     }
 
-    public final void setMigrationLoader(final Callable<LinkedHashMap<String, List<String>>> migration) {
+    public final void setMigrationLoader(final Callable<Map<String, List<String>>> migration) {
         this._migration = migration;
     }
 
     private void migrateDatabase(ServletContext servletContext, Path path, final DataSource dataSource, final String webappVersion,
                                  final String subVersion) throws Exception {
         _logger.info("  - Migration ...");
 
-        Connection conn = null;
-        Statement statement = null;
-        try {
-            conn = dataSource.getConnection();
-            statement = conn.createStatement();
+        try (Connection conn = dataSource.getConnection();
+             Statement statement = conn.createStatement()) {
+
             this.foundErrors = doMigration(webappVersion, subVersion, servletContext, path, conn, statement);
             conn.commit();
-        } finally {
-            try {
-                if (statement != null) {
-                    statement.close();
-                }
-            } finally {
-                if (conn != null) {
-                    conn.close();
-                }
-            }
+        } catch (Exception e) {
+            _logger.warning("  - Migration: Exception running migration for version: " + webappVersion + " subversion: "
+                + subVersion + ". " + e.getMessage());
+            throw e;
         }
     }
 
@@ -270,6 +257,7 @@ boolean doMigration(String webappVersion, String subVersion, ServletContext serv
 
     /**
      * Execute a Java database migration.
+     *
      * @param conn database connection.
      * @param file Java migration class name prefixed with JAVA_MIGRATION_PREFIX ("java:")
      * @return <code>true</code> if there is any error while executing the migration. <code>false</code> if there are no errors.
@@ -340,34 +328,24 @@ private Pair<String, String> getDatabaseVersion(Statement statement) throws SQLE
     }
 
     private String newLookup(Statement statement, String key) throws SQLException {
-        ResultSet results = null;
-        try {
-            final String newGetVersion = "SELECT value FROM Settings WHERE name = '" + key + "'";
-            results = statement.executeQuery(newGetVersion);
+        final String newGetVersion = "SELECT value FROM Settings WHERE name = '" + key + "'";
+
+        try (ResultSet results = statement.executeQuery(newGetVersion)) {
             if (results.next()) {
                 return results.getString(1);
             }
             return null;
-        } finally {
-            if (results != null) {
-                results.close();
-            }
         }
     }
 
     private String oldLookup(Statement statement, int key) throws SQLException {
-        ResultSet results = null;
-        try {
-            final String newGetVersion = "SELECT value FROM Settings WHERE id = " + key;
-            results = statement.executeQuery(newGetVersion);
+        final String newGetVersion = "SELECT value FROM Settings WHERE id = " + key;
+
+        try (ResultSet results = statement.executeQuery(newGetVersion)) {
             if (results.next()) {
                 return results.getString(1);
             }
             return null;
-        } finally {
-            if (results != null) {
-                results.close();
-            }
         }
     }
 
@@ -414,7 +392,7 @@ private int numDots(String number) {
         return num;
     }
 
-    public LinkedHashMap<String, List<String>> getMigrationConfig() throws Exception {
+    public Map<String, List<String>> getMigrationConfig() throws Exception {
         return _migration.call();
     }
 

web/src/main/webResources/WEB-INF/config-db/database_migration.xml

@@ -42,7 +42,7 @@
   <util:map id="migrationMap"
             map-class="java.util.LinkedHashMap"
             key-type="java.lang.String"
-            value-type="java.lang.String">
+            value-type="java.util.List">
     <entry key="2.4.3">
       <list>
         <value>WEB-INF/classes/setup/sql/migrate/v243/migrate-</value>
@@ -154,7 +154,7 @@
   <util:map id="dataMigrationMap"
             map-class="java.util.LinkedHashMap"
             key-type="java.lang.String"
-            value-type="java.lang.String">
+            value-type="java.util.List">
     <entry key="3.0.3">
       <list>
         <value>WEB-INF/classes/setup/sql/migrate/v303/migrate-</value>
@@ -204,12 +204,12 @@
     <entry key="3.4.2">
       <list>
         <value>WEB-INF/classes/setup/sql/migrate/v342/migrate-</value>
-        <value>java:v342.AdvancedSearchFormMigration</value>
       </list>
     </entry>
     <entry key="3.4.3">
       <list>
         <value>WEB-INF/classes/setup/sql/migrate/v343/migrate-</value>
+        <value>java:v343.AdvancedSearchFormMigration</value>
       </list>
     </entry>
   </util:map>

web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v342/AdvancedSearchFormMigration.java → web/src/main/webapp/WEB-INF/classes/setup/sql/migrate/v343/AdvancedSearchFormMigration.java

@@ -21,7 +21,7 @@
  * Rome - Italy. email: geonetwork@osgeo.org
  */
 
-package v342;
+package v343;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.annotations.VisibleForTesting;
@@ -47,43 +47,21 @@
 public class AdvancedSearchFormMigration extends JsonDatabaseMigration
     implements DatabaseMigrationTask {
 
-    @Override
-    public void update(Connection connection) throws SQLException {
-        try (PreparedStatement statement = connection.prepareStatement(
-            "SELECT * FROM settings WHERE name='ui/config'")) {
-            ResultSet rs = statement.executeQuery();
-
-            if (rs.next()) {
-                String currentSettingJson = rs.getString("value");
-                Map<String, String> fieldsToUpdate = new HashMap<>(1);
-                fieldsToUpdate.put("/mods/search/advancedSearchTemplate",
-                    "\"../../catalog/views/default/templates/advancedSearchForm/defaultAdvancedSearchForm.html\"");
+    private final String settingName = "ui/config";
 
-                String newSettingJson = super.insertOrUpdateField(currentSettingJson, fieldsToUpdate);
-                try (PreparedStatement updateStatement = connection.prepareStatement("UPDATE settings SET "
-                    + "value=? WHERE name='ui/config'")) {
-                    updateStatement.setString(1, newSettingJson);
-                    updateStatement.executeUpdate();
-                }
-            }
-        } catch (IOException e) {
-            Log.error(Geonet.GEONETWORK + ".databasemigration",
-                "Error in AdvancedSearchFormMigration. Cannot complete the "
-                    + "migration", e);
-            throw new DatabaseMigrationException(e);
-        }
+    @Override
+    protected Map<String, String> setUpNewSettingValues() {
+        Map<String, String> fieldsToUpdate = new HashMap<>(1);
+        fieldsToUpdate.put("/mods/search/advancedSearchTemplate",
+            "\"../../catalog/views/default/templates/advancedSearchForm/defaultAdvancedSearchForm.html\"");
+        return fieldsToUpdate;
+    }
 
+    @Override
+    protected String getSettingName() {
+        return settingName;
     }
 
-    /**
-     * Given a JSON string and a map of fields to update/insert and their values return an updated JSON string.
-     *
-     * @param currentSettingJson the JSON string to modify.
-     * @param fieldsToUpdate     a {@link Map} with the fields to update. Keys are in the form of nested fields separated by
-     *                           slash ("/") and starting by slash. Values are in form of JSON strings. For example
-     *                           <code>"\"my new value\""</code>
-     * @return a JSON string with updated/inserted values.
-     */
     @VisibleForTesting
     @Override
     protected String insertOrUpdateField(String currentSettingJson, Map<String, String> fieldsToUpdate) throws IOException {

web/src/test/java/v342/AdvancedSearchFormMigrationTest.java → web/src/test/java/v343/AdvancedSearchFormMigrationTest.java

@@ -21,7 +21,7 @@
  * Rome - Italy. email: geonetwork@osgeo.org
  */
 
-package v342;
+package v343;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -33,6 +33,7 @@
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.runners.MockitoJUnitRunner;
+import v343.AdvancedSearchFormMigration;
 
 import java.io.IOException;
 import java.sql.Connection;