Merge pull request #2893 from mtbc/trac-970-12317-dbpatch-and-map-value

fix #970, #12317: adjust dbpatch table; delete map values with triggers

components/dsl/resources/ome/dsl/psql-footer.vm

@@ -338,13 +338,42 @@ CREATE TRIGGER ${type.tableName()}_annotation_link_delete_trigger
 -- END #1390
 --
 
+--
+-- #12317 -- delete map property values along with their holders
+--
 
+#foreach($type in $types)
+#foreach($prop in $type.classProperties)
+#if( $prop.class.name == "ome.dsl.MapField" )
+CREATE FUNCTION ${type.tableName()}_${prop.name}_map_entry_delete_trigger_function() RETURNS "trigger" AS '
+BEGIN
+    DELETE FROM ${type.tableName()}_${prop.name}
+#if( $type.superclass && !$type.discriminator )
+        WHERE ${type.tableName()}_id = OLD.${type.typeToColumn($type.superclass)}_id;
+#else
+        WHERE ${type.tableName()}_id = OLD.id;
+#end
+    RETURN OLD;
+END;'
+LANGUAGE plpgsql;
 
+CREATE TRIGGER ${type.tableName()}_${prop.name}_map_entry_delete_trigger
+    BEFORE DELETE ON ${type.tableName()}
+    FOR EACH ROW
+    EXECUTE PROCEDURE ${type.tableName()}_${prop.name}_map_entry_delete_trigger_function();
+
+#end
+#end
+#end
+--
+-- done #12317
+--
 
 -- First, we install a unique constraint so that it is only possible
 -- to go from versionA/patchA to versionB/patchB once.
+-- message is included so that in-patch adjustments may still be noted in the table.
 --
-alter table dbpatch add constraint unique_dbpatch unique (currentVersion, currentPatch, previousVersion, previousPatch);
+alter table dbpatch add constraint unique_dbpatch unique (currentVersion, currentPatch, previousVersion, previousPatch, message);
 
 --
 -- Since this is a table that we will be using in DB-specific ways, we're also going

components/model/resources/ome/util/actions/default.properties

@@ -67,3 +67,7 @@ sql_action.update_pixels_path=update pixels set path = ? where id = ?
 sql_action.update_pixels_repo=update pixels set repo = ? where id = ?
 sql_action.user_groups=select g.name from experimentergroup g, groupexperimentermap m, experimenter e where omeName = ? and e.id = m.child and m.parent = g.id
 sql_action.user_id=select id from experimenter where omeName = ?
+sql_action.adjust_within_patch.start=insert into dbpatch (currentversion, currentpatch, previousversion, previouspatch, message) \
+  values (:version, :patch, :version, :patch, :message)
+sql_action.adjust_within_patch.end=update dbpatch set finished = clock_timestamp() where \
+  currentversion = :version and currentpatch = :patch and previousversion = :version and previouspatch = :patch and message = :message

components/model/src/ome/util/SqlAction.java

@@ -21,7 +21,6 @@
 import java.util.UUID;
 
 import ome.conditions.InternalException;
-import ome.model.IObject;
 import ome.model.core.Channel;
 import ome.model.internal.Details;
 import ome.model.internal.Permissions;
@@ -36,10 +35,11 @@
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
 import org.springframework.jdbc.core.namedparam.SqlParameterSource;
-import org.springframework.jdbc.core.simple.ParameterizedRowMapper;
 import org.springframework.jdbc.core.simple.SimpleJdbcOperations;
 import org.springframework.jdbc.core.simple.SimpleJdbcTemplate;
 
+import com.google.common.collect.ImmutableMap;
+
 /**
  * Single wrapper for all JDBC activities.
  *
@@ -452,8 +452,6 @@ void setPixelsNamePathRepo(long pixId, String name, String path,
      */
     Map<Long, byte[]> getShareData(List<Long> ids);
 
-    Integer deleteMapProperty(String table, String property, long id);
-
     //
     // Previously PgArrayHelper
     //
@@ -518,6 +516,24 @@ void setPixelsNamePathRepo(long pixId, String name, String path,
 
     int changeTablePermissionsForGroup(String table, Long id, Long internal);
 
+    /**
+     * Add a unique message to the DB patch table within the current patch.
+     * This method marks the start of the corresponding DB adjustment process.
+     * @param version the version of the current DB
+     * @param patch the patch of the current DB
+     * @param message the new message to note
+     */
+    void addMessageWithinDbPatchStart(String version, int patch, String message);
+
+    /**
+     * Add a unique message to the DB patch table within the current patch.
+     * This method marks the end of the corresponding DB adjustment process.
+     * @param version the version of the current DB
+     * @param patch the patch of the current DB
+     * @param message the new message to note
+     */
+    void addMessageWithinDbPatchEnd(String version, int patch, String message);
+
     //
     // End PgArrayHelper
     //
@@ -973,6 +989,20 @@ public void delCurrentEventLog(String key) {
 
         }
 
+        @Override
+        public void addMessageWithinDbPatchStart(String version, int patch, String message) {
+            final Map<String, Object> parameters =
+                    ImmutableMap.<String, Object>of("version", version, "patch", patch, "message", message);
+            _jdbc().update(_lookup("adjust_within_patch.start"), parameters);
+        }
+
+        @Override
+        public void addMessageWithinDbPatchEnd(String version, int patch, String message) {
+            final Map<String, Object> parameters =
+                    ImmutableMap.<String, Object>of("version", version, "patch", patch, "message", message);
+            _jdbc().update(_lookup("adjust_within_patch.end"), parameters);
+        }
+
         //
         // DISTINGUISHED NAME (DN)
         // These methods guarantee that an empty or whitespace only string

components/model/src/ome/util/actions/PostgresSqlAction.java

@@ -313,18 +313,6 @@ public Object doInConnection(java.sql.Connection connection)
         });
     }
 
-    public Integer deleteMapProperty(String table, String property, long id) {
-        final String sql = "DELETE FROM " + table + "_" + property + " WHERE " + table + "_id = " + id;
-        return _jdbc().getJdbcOperations().execute(new ConnectionCallback<Integer>() {
-            public Integer doInConnection(java.sql.Connection connection) throws SQLException {
-                final Statement statement = connection.createStatement();
-                final int deletes = statement.executeUpdate(sql);
-                statement.close();
-                return deletes;
-            }
-        });
-    }
-
     public Set<String> currentUserNames() {
         List<String> names = _jdbc().query(_lookup("current_user_names"),  //$NON-NLS-1$
                         new RowMapper<String>() {

components/server/resources/ome/services/startup.xml

@@ -53,6 +53,7 @@
      class="ome.services.util.DBEnumCheck"
      init-method="start" lazy-init="false">
      <constructor-arg ref="executor"/>
+     <constructor-arg ref="preferenceContext"/>
   </bean>
 
   <bean id="serverDirectoryCheck"

components/server/src/ome/services/delete/DeleteStep.java

@@ -25,7 +25,6 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.hibernate.Hibernate;
 import org.hibernate.Query;
 import org.hibernate.Session;
 import org.perf4j.StopWatch;
@@ -89,17 +88,6 @@ public void action(Callback cb, Session session, SqlAction sql, GraphOpts opts)
             filesetId = (Long) fsQb.query(session).uniqueResult();
         }
 
-        if (em.mayHaveMapProperties(iObjectType)) {
-            // Delete any map property entries so that the holder can then be deleted.
-            final Object proxy = session.createQuery("FROM " + iObjectType.getName() + " WHERE id = " + id).uniqueResult();
-            if (proxy != null) {
-                final Class<?> realClass = Hibernate.getClass(proxy);
-                for (final String property : em.getMapProperties(realClass.getName())) {
-                    sql.deleteMapProperty(realClass.getSimpleName(), property, id);
-                }
-            }
-        }
-
         // Phase 4: primary action
         StopWatch swStep = new Slf4JStopWatch();
         qb.param("id", id);

components/server/src/ome/services/util/BaseDBCheck.java

@@ -19,6 +19,7 @@
 
 package ome.services.util;
 
+import ome.system.PreferenceContext;
 import ome.util.SqlAction;
 
 import org.slf4j.Logger;
@@ -36,14 +37,21 @@ abstract class BaseDBCheck {
     /** executor useful for performing database adjustments */
     protected final Executor executor;
 
+    /* current database version */
+    private final String version;
+    private final int patch;
+
     private final String configKey = "DB check " + getClass().getSimpleName();
     private final String configValue = getCheckDone();
+    private final String configKeyValue = configKey + ": " + configValue;
 
     /**
      * @param executor executor to use for configuration map check
      */
-    protected BaseDBCheck(Executor executor) {
+    protected BaseDBCheck(Executor executor, PreferenceContext preferences) {
         this.executor = executor;
+        this.version = preferences.getProperty("omero.db.version");
+        this.patch = Integer.parseInt(preferences.getProperty("omero.db.patch"));
     }
 
     /**
@@ -59,6 +67,20 @@ public Boolean doWork(SqlAction sql) {
                 });
     }
 
+    /**
+     * The database adjustment is to be performed.
+     */
+    private void checkIsStarting() {
+        executor.executeSql(
+                new Executor.SimpleSqlWork(this, "BaseDBCheck") {
+                    @Transactional(readOnly = false)
+                    public Object doWork(SqlAction sql) {
+                        sql.addMessageWithinDbPatchStart(version, patch, configKeyValue);
+                        return null;
+                    }
+                });
+    }
+
     /**
      * The database adjustment is now performed.
      * Hereafter {@link #isCheckRequired()} should return {@code false}.
@@ -68,6 +90,7 @@ private void checkIsDone() {
                 new Executor.SimpleSqlWork(this, "BaseDBCheck") {
                     @Transactional(readOnly = false)
                     public Object doWork(SqlAction sql) {
+                        sql.addMessageWithinDbPatchEnd(version, patch, configKeyValue);
                         sql.updateOrInsertConfigValue(configKey, configValue);
                         return null;
                     }
@@ -79,9 +102,10 @@ public Object doWork(SqlAction sql) {
      */
     public void start() {
         if (isCheckRequired()) {
+            checkIsStarting();
             doCheck();
             checkIsDone();
-            log.info("performed " + configKey + ": " + configValue);
+            log.info("performed " + configKeyValue);
         } else if (log.isDebugEnabled()) {
             log.debug("skipped " + configKey);
         }

components/server/src/ome/services/util/DBEnumCheck.java

@@ -14,6 +14,7 @@
 import loci.formats.IFormatReader;
 import loci.formats.ImageReader;
 import ome.conditions.InternalException;
+import ome.system.PreferenceContext;
 import ome.util.SqlAction;
 
 import org.slf4j.Logger;
@@ -86,8 +87,8 @@ public static boolean shouldBeOmitted(String name) {
         return omitlist.contains(name);
     }
 
-    public DBEnumCheck(Executor executor) {
-        super(executor);
+    public DBEnumCheck(Executor executor, PreferenceContext preferences) {
+        super(executor, preferences);
     }
 
     @Override

etc/omero.properties

@@ -16,7 +16,7 @@ omero.managed.dir=${omero.data.dir}/ManagedRepository
 
 omero.db.authority=export.openmicroscopy.org
 omero.db.version=OMERO5.1DEV
-omero.db.patch=7
+omero.db.patch=8
 omero.db.host=localhost
 omero.db.name=omero
 omero.db.user=omero