[LMS-1866] improved performance of batch update of contained samples

SVN: 18613

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

@@ -580,38 +580,132 @@ abstract class AbstractSampleBusinessObject extends AbstractSampleIdentifierBusi
             Map<SampleOwnerIdentifier, SampleOwner> sampleOwnerCache)
     {
         assert sampleIdentifiers != null : "Sample identifiers unspecified.";
-        Map<SampleOwner, List<String>> samplesByOwner = new HashMap<SampleOwner, List<String>>();
+
+        Map<SampleOwnerWithContainer, List<String>> samplesByOwner =
+                new HashMap<SampleOwnerWithContainer, List<String>>();
         for (SampleIdentifier sampleIdentifier : sampleIdentifiers)
         {
             final SampleOwner sampleOwner = getSampleOwner(sampleOwnerCache, sampleIdentifier);
-            List<String> ownerSamples = samplesByOwner.get(sampleOwner);
+            final String containerCodeOrNull = sampleIdentifier.getContainerCodeOrNull();
+            final SampleOwnerWithContainer owner =
+                    new SampleOwnerWithContainer(sampleOwner, containerCodeOrNull);
+
+            List<String> ownerSamples = samplesByOwner.get(owner);
             if (ownerSamples == null)
             {
                 ownerSamples = new ArrayList<String>();
-                samplesByOwner.put(sampleOwner, ownerSamples);
+                samplesByOwner.put(owner, ownerSamples);
             }
-            ownerSamples.add(sampleIdentifier.getSampleCode());
+            ownerSamples.add(sampleIdentifier.getSampleSubCode());
         }
 
         final ISampleDAO sampleDAO = getSampleDAO();
         final List<SamplePE> results = new ArrayList<SamplePE>();
-        for (Entry<SampleOwner, List<String>> entry : samplesByOwner.entrySet())
+        for (Entry<SampleOwnerWithContainer, List<String>> entry : samplesByOwner.entrySet())
         {
-            SampleOwner sampleOwner = entry.getKey();
-            List<String> sampleCodes = entry.getValue();
+            final SampleOwnerWithContainer owner = entry.getKey();
+            final List<String> sampleCodes = entry.getValue();
+
+            final SampleOwner sampleOwner = owner.getSampleOwner();
+            final String containerCodeOrNull = owner.getContainerCodeOrNull();
+
             List<SamplePE> samples = null;
             if (sampleOwner.isDatabaseInstanceLevel())
             {
                 samples =
-                        sampleDAO.listByCodesAndDatabaseInstance(sampleCodes,
+                        sampleDAO.listByCodesAndDatabaseInstance(sampleCodes, containerCodeOrNull,
                                 sampleOwner.tryGetDatabaseInstance());
             } else
             {
                 assert sampleOwner.isGroupLevel() : "Must be of space level.";
-                samples = sampleDAO.listByCodesAndGroup(sampleCodes, sampleOwner.tryGetGroup());
+                samples =
+                        sampleDAO.listByCodesAndGroup(sampleCodes, containerCodeOrNull,
+                                sampleOwner.tryGetGroup());
             }
             results.addAll(samples);
         }
         return results;
     }
+
+    /** Helper class encapsulating {@link SampleOwner} and code of container of a sample. */
+    private static class SampleOwnerWithContainer
+    {
+        private final SampleOwner sampleOwner;
+
+        private final String containerCodeOrNull;
+
+        public SampleOwnerWithContainer(SampleOwner sampleOwner, String containerCodeOrNull)
+        {
+            assert sampleOwner != null;
+            this.sampleOwner = sampleOwner;
+            this.containerCodeOrNull = containerCodeOrNull;
+        }
+
+        public SampleOwner getSampleOwner()
+        {
+            return sampleOwner;
+        }
+
+        public String getContainerCodeOrNull()
+        {
+            return containerCodeOrNull;
+        }
+
+        //
+        // Object
+        //
+
+        @Override
+        public int hashCode()
+        {
+            final int prime = 31;
+            int result = 1;
+            result =
+                    prime * result
+                            + ((containerCodeOrNull == null) ? 0 : containerCodeOrNull.hashCode());
+            result = prime * result + sampleOwner.hashCode();
+            return result;
+        }
+
+        @Override
+        public boolean equals(Object obj)
+        {
+            if (this == obj)
+            {
+                return true;
+            }
+            if (obj == null)
+            {
+                return false;
+            }
+            if (!(obj instanceof SampleOwnerWithContainer))
+            {
+                return false;
+            }
+            SampleOwnerWithContainer other = (SampleOwnerWithContainer) obj;
+            if (containerCodeOrNull == null)
+            {
+                if (other.containerCodeOrNull != null)
+                {
+                    return false;
+                }
+            } else if (!containerCodeOrNull.equals(other.containerCodeOrNull))
+            {
+                return false;
+            }
+            if (!sampleOwner.equals(other.sampleOwner))
+            {
+                return false;
+            }
+            return true;
+        }
+
+        @Override
+        public String toString()
+        {
+            return "SampleOwnerWithContainer [sampleOwner=" + sampleOwner
+                    + ", containerCodeOrNull=" + containerCodeOrNull + "]";
+        }
+
+    }
 }

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

@@ -239,6 +239,7 @@ public final class SampleTable extends AbstractSampleBusinessObject implements I
         batchUpdateProperties(sample, updates.getProperties(), details.getPropertiesToUpdate());
         checkPropertiesBusinessRules(sample, propertiesCache);
 
+        // TODO 2010-10-09, Piotr Buczek: don't check business rules if value wasn't changed
         if (details.isExperimentUpdateRequested())
         {
             updateGroup(sample, updates.getSampleIdentifier(), sampleOwnerCache);
@@ -325,27 +326,13 @@ public final class SampleTable extends AbstractSampleBusinessObject implements I
             Map<SampleOwnerIdentifier, SampleOwner> sampleOwnerCache)
     {
         final List<SamplePE> results = new ArrayList<SamplePE>();
-        List<SampleIdentifier> identifiersToLoadInBatch = new ArrayList<SampleIdentifier>();
+        List<SampleIdentifier> identifiers = new ArrayList<SampleIdentifier>();
         for (SampleBatchUpdatesDTO sampleUpdates : updates)
         {
             SampleIdentifier identifier = sampleUpdates.getOldSampleIdentifierOrNull();
-            if (identifier.getSampleCode().contains(
-                    SampleIdentifier.CONTAINED_SAMPLE_CODE_SEPARARTOR_STRING))
-            {
-                // contained samples can't be loaded in batch
-                SamplePE sample = tryToGetSampleByIdentifier(identifier);
-                if (sample == null)
-                {
-                    throw UserFailureException.fromTemplate(
-                            "No sample could be found with given identifier '%s'.", identifier);
-                }
-                results.add(sample);
-            } else
-            {
-                identifiersToLoadInBatch.add(identifier);
-            }
+            identifiers.add(identifier);
         }
-        results.addAll(listSamplesByIdentifiers(identifiersToLoadInBatch, sampleOwnerCache));
+        results.addAll(listSamplesByIdentifiers(identifiers, sampleOwnerCache));
         return results;
     }
 

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

@@ -54,9 +54,12 @@ public interface ISampleDAO extends IGenericDAO<SamplePE>
 
     /**
      * Returns a list of samples with given <var>databaseInstance</var> and one of given codes.
+     * 
+     * @param containerCodeOrNull if specified all returned samples should have container with
+     *            specified code, otherwise they shouldn't have any container
      */
     List<SamplePE> listByCodesAndDatabaseInstance(final List<String> sampleCodes,
-            final DatabaseInstancePE databaseInstance);
+            String containerCodeOrNull, final DatabaseInstancePE databaseInstance);
 
     /**
      * Returns the sample specified by given <var>sampleCode</var> and given <var>group</var>.
@@ -66,8 +69,12 @@ public interface ISampleDAO extends IGenericDAO<SamplePE>
 
     /**
      * Returns a list of samples with given <var>group</var> and one of given codes.
+     * 
+     * @param containerCodeOrNull if specified all returned samples should have container with
+     *            specified code, otherwise they shouldn't have any container
      */
-    List<SamplePE> listByCodesAndGroup(final List<String> sampleCodes, final GroupPE group);
+    List<SamplePE> listByCodesAndGroup(final List<String> sampleCodes, String containerCodeOrNull,
+            final GroupPE group);
 
     /**
      * Inserts given list of {@link SamplePE} into the database in one go.

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

@@ -207,13 +207,13 @@ public class SampleDAO extends AbstractGenericEntityWithPropertiesDAO<SamplePE>
     }
 
     public final List<SamplePE> listByCodesAndDatabaseInstance(final List<String> sampleCodes,
-            final DatabaseInstancePE databaseInstance)
+            final String containerCodeOrNull, final DatabaseInstancePE databaseInstance)
     {
         assert sampleCodes != null : "Unspecified sample codes.";
         assert databaseInstance != null : "Unspecified database instance.";
 
         Criteria criteria = createDatabaseInstanceCriteria(databaseInstance);
-        addSampleCodesCriterion(criteria, sampleCodes);
+        addSampleCodesCriterion(criteria, sampleCodes, containerCodeOrNull);
         List<SamplePE> result = cast(criteria.list());
         if (operationLog.isDebugEnabled())
         {
@@ -245,13 +245,13 @@ public class SampleDAO extends AbstractGenericEntityWithPropertiesDAO<SamplePE>
     }
 
     public final List<SamplePE> listByCodesAndGroup(final List<String> sampleCodes,
-            final GroupPE group)
+            final String containerCodeOrNull, final GroupPE group)
     {
         assert sampleCodes != null : "Unspecified sample codes.";
         assert group != null : "Unspecified space.";
 
         Criteria criteria = createGroupCriteria(group);
-        addSampleCodesCriterion(criteria, sampleCodes);
+        addSampleCodesCriterion(criteria, sampleCodes, containerCodeOrNull);
         List<SamplePE> result = cast(criteria.list());
         if (operationLog.isDebugEnabled())
         {
@@ -267,7 +267,7 @@ public class SampleDAO extends AbstractGenericEntityWithPropertiesDAO<SamplePE>
 
     private SamplePE tryFindContainedSampleWithUniqueSubcode(Criteria criteria, String sampleCode)
     {
-        addContainedSampleCodeCriterion(criteria, sampleCode);
+        addContainedCodeCriterion(criteria, sampleCode);
         List<SamplePE> list = cast(criteria.list());
         return list.size() == 1 ? list.get(0) : null;
     }
@@ -290,32 +290,41 @@ public class SampleDAO extends AbstractGenericEntityWithPropertiesDAO<SamplePE>
         return createFindCriteria(Restrictions.eq("group", group));
     }
 
-    private void addSampleCodesCriterion(Criteria criteria, List<String> sampleCodes)
+    private void addSampleCodesCriterion(Criteria criteria, List<String> sampleCodes,
+            String containerCodeOrNull)
     {
-        // no support for contained samples
-        criteria.add(Restrictions.in("code", sampleCodes));
-        criteria.add(Restrictions.isNull("container"));
+        List<String> convertedCodes = new ArrayList<String>();
+        for (String sampleCode : sampleCodes)
+        {
+            convertedCodes.add(CodeConverter.tryToDatabase(sampleCode));
+        }
+        criteria.add(Restrictions.in("code", convertedCodes));
+        addSampleContainerCriterion(criteria, containerCodeOrNull);
     }
 
-    private void addSampleCodeCriterion(Criteria criteria, String sampleCode)
+    private void addSampleContainerCriterion(Criteria criteria, String containerCodeOrNull)
     {
-        String[] sampleCodeTokens =
-                sampleCode.split(SampleIdentifier.CONTAINED_SAMPLE_CODE_SEPARARTOR_STRING);
-        if (sampleCodeTokens.length > 1)
+        if (containerCodeOrNull != null)
         {
-            final String containerCode = sampleCodeTokens[0];
-            final String code = sampleCodeTokens[1];
-            criteria.add(Restrictions.eq("code", CodeConverter.tryToDatabase(code)));
             criteria.createAlias("container", "c");
-            criteria.add(Restrictions.eq("c.code", CodeConverter.tryToDatabase(containerCode)));
+            criteria.add(Restrictions.eq("c.code", CodeConverter.tryToDatabase(containerCodeOrNull)));
         } else
         {
-            criteria.add(Restrictions.eq("code", CodeConverter.tryToDatabase(sampleCode)));
             criteria.add(Restrictions.isNull("container"));
         }
     }
 
-    private void addContainedSampleCodeCriterion(Criteria criteria, String sampleCode)
+    private void addSampleCodeCriterion(Criteria criteria, String sampleCode)
+    {
+        String[] sampleCodeTokens =
+                sampleCode.split(SampleIdentifier.CONTAINED_SAMPLE_CODE_SEPARARTOR_STRING);
+        String subCode = sampleCodeTokens.length > 1 ? sampleCodeTokens[1] : sampleCode;
+        String containerCodeOrNull = sampleCodeTokens.length > 1 ? sampleCodeTokens[0] : null;
+        criteria.add(Restrictions.eq("code", CodeConverter.tryToDatabase(subCode)));
+        addContainedCodeCriterion(criteria, containerCodeOrNull);
+    }
+
+    private void addContainedCodeCriterion(Criteria criteria, String sampleCode)
     {
         criteria.add(Restrictions.eq("code", CodeConverter.tryToDatabase(sampleCode)));
         criteria.add(Restrictions.isNotNull("container"));

openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifier.java

@@ -37,6 +37,8 @@ public class SampleIdentifier extends SampleOwnerIdentifier
 
     private String sampleSubCode;
 
+    private String containerCodeOrNull;
+
     private SampleIdentifier(final DatabaseInstanceIdentifier databaseInstanceIdentOrNull,
             final SpaceIdentifier spaceIdentOrNull, final String sampleCode)
     {
@@ -88,6 +90,11 @@ public class SampleIdentifier extends SampleOwnerIdentifier
         return sampleSubCode;
     }
 
+    public String getContainerCodeOrNull()
+    {
+        return containerCodeOrNull;
+    }
+
     /**
      * Returns an object that only contains the owner information of this sample identifier.
      * {@link #hashCode()} will be the same when called on two different samples with same owner.
@@ -111,6 +118,10 @@ public class SampleIdentifier extends SampleOwnerIdentifier
         {
             String[] sampleCodeTokens = sampleCode.split(CONTAINED_SAMPLE_CODE_SEPARARTOR_STRING);
             this.sampleSubCode = sampleCodeTokens[sampleCodeTokens.length - 1];
+            if (sampleCodeTokens.length > 1)
+            {
+                containerCodeOrNull = sampleCodeTokens[0];
+            }
         } else
         {
             this.sampleSubCode = sampleCode;