[LMS-1585] improved speed of deletion of samples and creation of deletion...

[LMS-1585] improved speed of deletion of samples and creation of deletion events in DAO (authorization is still slow, no tests)

SVN: 16656

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java

@@ -54,6 +54,7 @@ import ch.systemsx.cisd.openbis.generic.server.business.bo.IPropertyTypeBO;
 import ch.systemsx.cisd.openbis.generic.server.business.bo.IPropertyTypeTable;
 import ch.systemsx.cisd.openbis.generic.server.business.bo.IRoleAssignmentTable;
 import ch.systemsx.cisd.openbis.generic.server.business.bo.ISampleBO;
+import ch.systemsx.cisd.openbis.generic.server.business.bo.ISampleTable;
 import ch.systemsx.cisd.openbis.generic.server.business.bo.IVocabularyBO;
 import ch.systemsx.cisd.openbis.generic.server.business.bo.IVocabularyTermBO;
 import ch.systemsx.cisd.openbis.generic.server.business.bo.datasetlister.IDatasetLister;
@@ -1008,11 +1009,8 @@ public final class CommonServer extends AbstractCommonServer<ICommonServer> impl
         Session session = getSession(sessionToken);
         try
         {
-            ISampleBO sampleBO = businessObjectFactory.createSampleBO(session);
-            for (TechId id : sampleIds)
-            {
-                sampleBO.deleteByTechId(id, reason);
-            }
+            ISampleTable sampleTableBO = businessObjectFactory.createSampleTable(session);
+            sampleTableBO.deleteByTechIds(sampleIds, reason);
         } catch (final DataAccessException ex)
         {
             throw createUserFailureException(ex);

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ISampleBO.java

@@ -96,11 +96,4 @@ public interface ISampleBO extends IEntityBusinessObject
     /** Adds attachments */
     void enrichWithAttachments();
 
-    /**
-     * Deletes sample for specified reason.
-     * 
-     * @param sampleId sample technical identifier
-     * @throws UserFailureException if sample with given technical identifier is not found.
-     */
-    public void deleteByTechId(TechId sampleId, String reason) throws UserFailureException;
 }

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ISampleTable.java

@@ -20,6 +20,7 @@ import java.util.List;
 
 import ch.systemsx.cisd.common.exceptions.UserFailureException;
 import ch.systemsx.cisd.openbis.generic.server.business.bo.samplelister.ISampleLister;
+import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ListSampleCriteriaDTO;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ListSamplesByPropertyCriteria;
@@ -71,4 +72,12 @@ public interface ISampleTable
 
     void update(List<SampleBatchUpdatesDTO> updates);
 
+    /**
+     * Deletes samples for specified reason.
+     * 
+     * @param sampleIds sample technical identifiers
+     * @throws UserFailureException if one of the samples can not be deleted.
+     */
+    public void deleteByTechIds(List<TechId> sampleIds, String reason) throws UserFailureException;
+
 }

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleBO.java

@@ -33,15 +33,12 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewAttachment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
 import ch.systemsx.cisd.openbis.generic.shared.dto.AttachmentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.EntityTypePE;
-import ch.systemsx.cisd.openbis.generic.shared.dto.EventPE;
-import ch.systemsx.cisd.openbis.generic.shared.dto.EventType;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePropertyPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SampleUpdatesDTO;
 import ch.systemsx.cisd.openbis.generic.shared.dto.Session;
-import ch.systemsx.cisd.openbis.generic.shared.dto.EventPE.EntityType;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.IdentifierHelper;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifier;
 import ch.systemsx.cisd.openbis.generic.shared.dto.properties.EntityKind;
@@ -127,37 +124,6 @@ public final class SampleBO extends AbstractSampleBusinessObject implements ISam
         }
     }
 
-    public void deleteByTechId(TechId sampleId, String reason) throws UserFailureException
-    {
-        loadDataByTechId(sampleId);
-        try
-        {
-            getSampleDAO().delete(sample);
-            getEventDAO().persist(createDeletionEvent(sample, session.tryGetPerson(), reason));
-        } catch (final DataAccessException ex)
-        {
-            throwException(ex, String.format("Sample '%s'", sample.getCode()), EntityKind.SAMPLE);
-        }
-    }
-
-    public static EventPE createDeletionEvent(SamplePE sample, PersonPE registrator, String reason)
-    {
-        EventPE event = new EventPE();
-        event.setEventType(EventType.DELETION);
-        event.setEntityType(EntityType.SAMPLE);
-        event.setIdentifier(sample.getPermId());
-        event.setDescription(getDeletionDescription(sample));
-        event.setReason(reason);
-        event.setRegistrator(registrator);
-
-        return event;
-    }
-
-    private static String getDeletionDescription(SamplePE sample)
-    {
-        return String.format("%s [%s]", sample.getIdentifier(), sample.getPermId());
-    }
-
     public final void define(final NewSample newSample)
     {
         assert newSample != null : "Unspecified new sample.";

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleTable.java

@@ -51,6 +51,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.LocalExperimentIde
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ProjectIdentifier;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifier;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleOwnerIdentifier;
+import ch.systemsx.cisd.openbis.generic.shared.dto.properties.EntityKind;
 import ch.systemsx.cisd.openbis.generic.shared.util.HibernateUtils;
 
 /**
@@ -423,4 +424,17 @@ public final class SampleTable extends AbstractSampleBusinessObject implements I
         setBatchUpdateMode(false);
     }
 
+    public void deleteByTechIds(List<TechId> sampleIds, String reason) throws UserFailureException
+    {
+        try
+        {
+            getSessionFactory().getCurrentSession().flush();
+            getSessionFactory().getCurrentSession().clear();
+            getSampleDAO().delete(sampleIds, session.tryGetPerson(), reason);
+        } catch (final DataAccessException ex)
+        {
+            throwException(ex, "Samples", EntityKind.SAMPLE);
+        }
+    }
+
 }

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/ISampleDAO.java

@@ -21,9 +21,11 @@ import java.util.Set;
 
 import org.springframework.dao.DataAccessException;
 
+import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SampleTypePE;
 
@@ -34,7 +36,6 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.SampleTypePE;
  */
 public interface ISampleDAO extends IGenericDAO<SamplePE>
 {
-
     /**
      * Inserts given {@link SamplePE} into the database.
      */
@@ -120,6 +121,13 @@ public interface ISampleDAO extends IGenericDAO<SamplePE>
     List<SamplePE> listSamplesByGroupAndProperty(final String propertyCode,
             final String propertyValue, final GroupPE group) throws DataAccessException;
 
+	/**
+     * Lists samples (with minimal additional information) with permanent identifier in given set of values.
+     */
     List<SamplePE> listByPermID(Set<String> values);
 
+    /**
+     * Delete samples with given by specified registrator with specified reason.
+     */
+    void delete(List<TechId> sampleIds, PersonPE registrator, String reason) throws DataAccessException;
 }

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/AuthorizationDAOFactory.java

@@ -42,6 +42,7 @@ import ch.systemsx.cisd.openbis.generic.server.dataaccess.IQueryDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IRoleAssignmentDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.ISampleDAO;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.PersistencyResources;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.search.IFullTextIndexUpdateScheduler;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
 import ch.systemsx.cisd.openbis.generic.shared.util.UuidUtil;
 
@@ -82,7 +83,8 @@ public class AuthorizationDAOFactory implements IAuthorizationDAOFactory
     private final PersistencyResources persistencyResources;
 
     public AuthorizationDAOFactory(final DatabaseConfigurationContext context,
-            final SessionFactory sessionFactory)
+            final SessionFactory sessionFactory,
+            final IFullTextIndexUpdateScheduler indexUpdateScheduler)
     {
         persistencyResources = new PersistencyResources(context, sessionFactory);
         databaseInstancesDAO = new DatabaseInstanceDAO(sessionFactory);
@@ -93,7 +95,7 @@ public class AuthorizationDAOFactory implements IAuthorizationDAOFactory
         externalDataDAO = new ExternalDataDAO(sessionFactory, homeDatabaseInstance);
         experimentDAO = new ExperimentDAO(sessionFactory, homeDatabaseInstance);
         projectDAO = new ProjectDAO(sessionFactory, homeDatabaseInstance);
-        sampleDAO = new SampleDAO(sessionFactory, homeDatabaseInstance);
+        sampleDAO = new SampleDAO(sessionFactory, homeDatabaseInstance, indexUpdateScheduler);
         gridCustomFilterDAO = new GridCustomFilterDAO(sessionFactory, homeDatabaseInstance);
         gridCustomColumnDAO = new GridCustomColumnDAO(sessionFactory, homeDatabaseInstance);
         queryDAO = new QueryDAO(sessionFactory, homeDatabaseInstance);

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/DAOFactory.java

@@ -90,7 +90,7 @@ public final class DAOFactory extends AuthorizationDAOFactory implements IDAOFac
             final SessionFactory sessionFactory,
             final IFullTextIndexUpdateScheduler fullTextIndexUpdateScheduler)
     {
-        super(context, sessionFactory);
+        super(context, sessionFactory, fullTextIndexUpdateScheduler);
         final DatabaseInstancePE databaseInstance = getHomeDatabaseInstance();
         sampleTypeDAO = new SampleTypeDAO(sessionFactory, databaseInstance);
         hibernateSearchDAO = new HibernateSearchDAO(sessionFactory);

openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/SampleDAO.java

@@ -24,7 +24,9 @@ import java.util.Set;
 import org.apache.log4j.Logger;
 import org.hibernate.Criteria;
 import org.hibernate.FetchMode;
+import org.hibernate.SQLQuery;
 import org.hibernate.SessionFactory;
+import org.hibernate.StatelessSession;
 import org.hibernate.criterion.Criterion;
 import org.hibernate.criterion.DetachedCriteria;
 import org.hibernate.criterion.Restrictions;
@@ -39,13 +41,21 @@ import ch.systemsx.cisd.common.logging.LogCategory;
 import ch.systemsx.cisd.common.logging.LogFactory;
 import ch.systemsx.cisd.common.utilities.ExceptionUtils;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.ISampleDAO;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.search.IFullTextIndexUpdateScheduler;
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.search.IndexUpdateOperation;
+import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
 import ch.systemsx.cisd.openbis.generic.shared.dto.CodeConverter;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.EventType;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePropertyPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SampleTypePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.SequenceNames;
+import ch.systemsx.cisd.openbis.generic.shared.dto.TableNames;
+import ch.systemsx.cisd.openbis.generic.shared.dto.EventPE.EntityType;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifier;
 
 /**
@@ -64,9 +74,13 @@ public class SampleDAO extends AbstractGenericEntityDAO<SamplePE> implements ISa
     private static final Logger operationLog =
             LogFactory.getLogger(LogCategory.OPERATION, SampleDAO.class);
 
-    SampleDAO(final SessionFactory sessionFactory, final DatabaseInstancePE databaseInstance)
+    private final IFullTextIndexUpdateScheduler fullTextIndexUpdateScheduler;
+
+    SampleDAO(final SessionFactory sessionFactory, final DatabaseInstancePE databaseInstance,
+            final IFullTextIndexUpdateScheduler fullTextIndexUpdateScheduler)
     {
         super(sessionFactory, databaseInstance, SamplePE.class);
+        this.fullTextIndexUpdateScheduler = fullTextIndexUpdateScheduler;
     }
 
     private final Criteria createListAllSamplesCriteria()
@@ -516,4 +530,82 @@ public class SampleDAO extends AbstractGenericEntityDAO<SamplePE> implements ISa
         }
     }
 
+    public void delete(final List<TechId> sampleIds, final PersonPE registrator, final String reason)
+            throws DataAccessException
+    {
+        final String sqlPermId = "SELECT perm_id FROM samples WHERE id = :s_id";
+        final String sqlInsertEvent =
+                String
+                        .format(
+                                "INSERT INTO %s (id, event_type, description, reason, pers_id_registerer, entity_type, identifier) "
+                                        + "VALUES (nextval('%s'), :eventType, :description, :reason, :registratorId, :entityType, :identifier)",
+                                TableNames.EVENTS_TABLE, SequenceNames.EVENT_SEQUENCE);
+        final String sqlDeleteProperties = "DELETE FROM sample_properties WHERE samp_id = :s_id";
+        final String sqlDeleteSample = "DELETE FROM samples WHERE id = :s_id";
+
+        executeStatelessAction(new StatelessHibernateCallback()
+            {
+                public Object doInStatelessSession(StatelessSession session)
+                {
+                    final SQLQuery sqlQueryPermId = session.createSQLQuery(sqlPermId);
+                    final SQLQuery sqlQueryInsertEvent = session.createSQLQuery(sqlInsertEvent);
+                    sqlQueryInsertEvent.setParameter("eventType", EventType.DELETION.name());
+                    sqlQueryInsertEvent.setParameter("reason", reason);
+                    sqlQueryInsertEvent.setParameter("registratorId", registrator.getId());
+                    sqlQueryInsertEvent.setParameter("entityType", EntityType.SAMPLE.name());
+                    final SQLQuery sqlQueryDeleteProperties =
+                            session.createSQLQuery(sqlDeleteProperties);
+                    final SQLQuery sqlQueryDeleteSample = session.createSQLQuery(sqlDeleteSample);
+                    int counter = 0;
+                    // insertion of events is separated for better performance debugging
+                    List<String> permIds = new ArrayList<String>();
+                    for (TechId techId : sampleIds)
+                    {
+                        sqlQueryPermId.setParameter("s_id", techId.getId());
+                        final String permIdOrNull = tryGetEntity(sqlQueryPermId.uniqueResult());
+                        if (permIdOrNull != null)
+                        {
+                            permIds.add(permIdOrNull);
+                            sqlQueryDeleteProperties.setParameter("s_id", techId.getId());
+                            sqlQueryDeleteProperties.executeUpdate();
+                            sqlQueryDeleteSample.setParameter("s_id", techId.getId());
+                            sqlQueryDeleteSample.executeUpdate();
+                            if (++counter % 100 == 0)
+                            {
+                                operationLog.info(String.format("%d samples have been deleted...",
+                                        counter));
+                            }
+                        }
+                    }
+                    counter = 0;
+                    for (String permId : permIds)
+                    {
+                        sqlQueryInsertEvent.setParameter("description", permId);
+                        sqlQueryInsertEvent.setParameter("identifier", permId);
+                        sqlQueryInsertEvent.executeUpdate();
+                        if (++counter % 100 == 0)
+                        {
+                            operationLog.info(String.format("%d events have been created...",
+                                    counter));
+                        }
+                    }
+                    return null;
+                }
+            });
+
+        // index will not be updated automatically by Hibernate because we use native SQL queries
+        scheduleRemoveFromFullTextIndex(sampleIds);
+    }
+
+    private void scheduleRemoveFromFullTextIndex(List<TechId> sampleIds)
+    {
+        List<Long> ids = new ArrayList<Long>();
+        for (TechId techId : sampleIds)
+        {
+            ids.add(techId.getId());
+        }
+        fullTextIndexUpdateScheduler.scheduleUpdate(IndexUpdateOperation
+                .remove(SamplePE.class, ids));
+    }
+
 }

openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/SampleBOTest.java

@@ -47,11 +47,9 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.PropertyType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.SampleType;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
-import ch.systemsx.cisd.openbis.generic.shared.dto.EventPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExternalDataPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
-import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePropertyPE;
@@ -302,28 +300,29 @@ public final class SampleBOTest extends AbstractBOTest
         context.assertIsSatisfied();
     }
 
-    @Test
-    public void testDelete()
-    {
-        final TechId sampleId = SAMPLE_TECH_ID;
-        final SamplePE sample = createSample(DEFAULT_SAMPLE_CODE);
-        sample.setId(sampleId.getId());
-        final String reason = "reason";
-
-        prepareTryToLoadOfSampleWithId(sample);
-        context.checking(new Expectations()
-            {
-                {
-                    PersonPE person = EXAMPLE_SESSION.tryGetPerson();
-                    EventPE event = SampleBO.createDeletionEvent(sample, person, reason);
-                    one(eventDAO).persist(event);
-                    one(sampleDAO).delete(sample);
-                }
-            });
-        final SampleBO sampleBO = createSampleBO();
-        sampleBO.deleteByTechId(sampleId, reason);
-        context.assertIsSatisfied();
-    }
+    // FIXME 2009-06-22, Piotr Buczek
+    // @Test
+    // public void testDelete()
+    // {
+    // final TechId sampleId = SAMPLE_TECH_ID;
+    // final SamplePE sample = createSample(DEFAULT_SAMPLE_CODE);
+    // sample.setId(sampleId.getId());
+    // final String reason = "reason";
+    //
+    // prepareTryToLoadOfSampleWithId(sample);
+    // context.checking(new Expectations()
+    // {
+    // {
+    // PersonPE person = EXAMPLE_SESSION.tryGetPerson();
+    // EventPE event = SampleBO.createDeletionEvent(sample, person, reason);
+    // one(eventDAO).persist(event);
+    // one(sampleDAO).delete(sample);
+    // }
+    // });
+    // final SampleBO sampleBO = createSampleBO();
+    // sampleBO.deleteByTechId(sampleId, reason);
+    // context.assertIsSatisfied();
+    // }
 
     @Test
     public final void testDetachFromExperiment()