SSDM-1081: Bug fixed concerning closing EDOSQL Query object. Better error...

SSDM-1081: Bug fixed concerning closing EDOSQL Query object. Better error message if data sets to unarchive are in more than one container.

SVN: 32718

datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/MultiDataSetArchiver.java

@@ -18,10 +18,12 @@ package ch.systemsx.cisd.openbis.dss.generic.server.plugins.standard.archiver;
 
 import java.io.File;
 import java.io.Serializable;
-import java.util.HashSet;
+import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.Properties;
 
 import ch.rinn.restrictions.Private;
@@ -123,14 +125,12 @@ public class MultiDataSetArchiver extends AbstractArchiverProcessingPlugin
             result.addResults(archiveResult);
 
             getTransaction().commit();
-            getTransaction().close();
         } catch (Exception e)
         {
             operationLog.warn("Archiving of " + dataSets.size() + " data sets failed", e);
             try
             {
                 getTransaction().rollback();
-                getTransaction().close();
             } catch (Exception ex)
             {
                 operationLog.warn("Rollback of multi dataset db transaction failed", ex);
@@ -412,7 +412,7 @@ public class MultiDataSetArchiver extends AbstractArchiverProcessingPlugin
      */
     private long assertAllDataSetsInTheSameContainer(List<String> dataSetCodes)
     {
-        HashSet<Long> containerIds = new HashSet<Long>();
+        Map<Long, List<String>> containers = new LinkedHashMap<Long, List<String>>();
         long containerId = -1;
         for (String code : dataSetCodes)
         {
@@ -422,13 +422,19 @@ public class MultiDataSetArchiver extends AbstractArchiverProcessingPlugin
                 throw new IllegalArgumentException("Dataset " + code
                         + " was selected for unarchiving, but is not present in the archive");
             }
-            containerIds.add(dataSet.getContainerId());
+            List<String> list = containers.get(dataSet.getContainerId());
+            if (list == null)
+            {
+                list = new ArrayList<String>();
+                containers.put(dataSet.getContainerId(), list);
+            }
+            list.add(dataSet.getCode());
             containerId = dataSet.getContainerId();
         }
-        if (containerIds.size() > 1)
+        if (containers.size() > 1)
         {
-            throw new IllegalArgumentException("Datasets selected for unarchiving do not all belong to one container, but to " + containerIds.size()
-                    + " different containers");
+            throw new IllegalArgumentException("Datasets selected for unarchiving do not all belong to one container, "
+                    + "but to " + containers.size() + " different containers: " + containers);
         }
         return containerId;
     }

datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/dataaccess/IMultiDataSetArchiverDBTransaction.java

@@ -38,19 +38,8 @@ public interface IMultiDataSetArchiverDBTransaction
 
     public MultiDataSetArchiverDataSetDTO getDataSetForCode(String code);
 
-    /**
-     * @see net.lemnik.eodsql.TransactionQuery#commit()
-     */
     public void commit();
 
-    /**
-     * @see net.lemnik.eodsql.TransactionQuery#rollback()
-     */
     public void rollback();
 
-    /**
-     * @see net.lemnik.eodsql.BaseQuery#close()
-     */
-    public void close();
-
 }
\ No newline at end of file

datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/dataaccess/MultiDataSetArchiverDBTransaction.java

@@ -100,13 +100,4 @@ public class MultiDataSetArchiverDBTransaction implements IMultiDataSetArchiverD
         transaction.rollback();
     }
 
-    /**
-     * @see net.lemnik.eodsql.BaseQuery#close()
-     */
-    @Override
-    public void close()
-    {
-        transaction.close();
-    }
-
 }

datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/MultiDataSetArchiverTest.java

@@ -111,8 +111,6 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
 
         private boolean rolledBack;
 
-        private boolean closed;
-
         @Override
         public List<MultiDataSetArchiverDataSetDTO> getDataSetsForContainer(MultiDataSetArchiverContainerDTO container)
         {
@@ -170,12 +168,6 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
             rolledBack = true;
         }
 
-        @Override
-        public void close()
-        {
-            closed = true;
-        }
-
         @Override
         public String toString()
         {
@@ -192,7 +184,6 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
             }
             builder.append("\ncomitted: ").append(committed);
             builder.append(", rolledBack: ").append(rolledBack);
-            builder.append(", closed: ").append(closed);
             return builder.toString();
         }
 
@@ -262,10 +253,6 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
 
     private DatasetDescription ds2;
 
-    private DatasetDescription ds3;
-
-    private DatasetDescription ds4;
-
     private File store;
 
     private File share;
@@ -314,8 +301,6 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
         archive.mkdirs();
         ds1 = dataSet("ds1", "0123456789");
         ds2 = dataSet("ds2", "01234567890123456789");
-        ds3 = dataSet("ds3", "012345678901234567890123456789");
-        ds4 = dataSet("ds4", "0123456789012345678901234567890123456789");
         properties = new Properties();
         properties.setProperty(STAGING_DESTINATION_KEY, staging.getAbsolutePath());
         properties.setProperty(FINAL_DESTINATION_KEY, archive.getAbsolutePath());
@@ -368,7 +353,7 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
                 + "to be archived with multi dataset archiver because minimum size is 35 bytes.\"]",
                 status.getErrorStatuses().toString());
         assertEquals("[ds1, ds2]: AVAILABLE false\n", statusUpdater.toString());
-        assertEquals("Containers:\nData sets:\ncomitted: false, rolledBack: true, closed: true", transaction.toString());
+        assertEquals("Containers:\nData sets:\ncomitted: false, rolledBack: true", transaction.toString());
         context.assertIsSatisfied();
     }
 
@@ -387,7 +372,7 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
                 + "to be archived with multi dataset archiver because maximum size is 27 bytes.\"]",
                 status.getErrorStatuses().toString());
         assertEquals("[ds1, ds2]: AVAILABLE false\n", statusUpdater.toString());
-        assertEquals("Containers:\nData sets:\ncomitted: false, rolledBack: true, closed: true", transaction.toString());
+        assertEquals("Containers:\nData sets:\ncomitted: false, rolledBack: true", transaction.toString());
         context.assertIsSatisfied();
     }
 
@@ -431,7 +416,7 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
         assertEquals("[ds2]: AVAILABLE true\n", statusUpdater.toString());
         assertEquals("Containers:\nMultiDataSetArchiverContainerDTO [id=0, path=ds2.tar]\n"
                 + "Data sets:\nMultiDataSetArchiverDataSetDTO [id=1, code=ds2, containerId=0, sizeInBytes=20]\n"
-                + "comitted: true, rolledBack: false, closed: true", transaction.toString());
+                + "comitted: true, rolledBack: false", transaction.toString());
         context.assertIsSatisfied();
     }
 
@@ -497,7 +482,7 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
         assertEquals("Containers:\nMultiDataSetArchiverContainerDTO [id=0, path=ds1.tar]\n"
                 + "Data sets:\nMultiDataSetArchiverDataSetDTO [id=1, code=ds1, containerId=0, sizeInBytes=10]\n"
                 + "MultiDataSetArchiverDataSetDTO [id=2, code=ds2, containerId=0, sizeInBytes=20]\n"
-                + "comitted: true, rolledBack: false, closed: true", transaction.toString());
+                + "comitted: true, rolledBack: false", transaction.toString());
         context.assertIsSatisfied();
     }
 
@@ -520,7 +505,7 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
         assertEquals("[ds2]: AVAILABLE true\n", statusUpdater.toString());
         assertEquals("Containers:\nMultiDataSetArchiverContainerDTO [id=0, path=path]\n"
                 + "Data sets:\nMultiDataSetArchiverDataSetDTO [id=1, code=ds2, containerId=0, sizeInBytes=20]\n"
-                + "comitted: false, rolledBack: false, closed: false", transaction.toString());
+                + "comitted: false, rolledBack: false", transaction.toString());
         context.assertIsSatisfied();
     }
 
@@ -570,7 +555,7 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
                 + "MultiDataSetArchiverContainerDTO [id=2, path=ds1.tar]\n"
                 + "Data sets:\nMultiDataSetArchiverDataSetDTO [id=1, code=ds2, containerId=0, sizeInBytes=20]\n"
                 + "MultiDataSetArchiverDataSetDTO [id=3, code=ds1, containerId=2, sizeInBytes=10]\n"
-                + "comitted: true, rolledBack: false, closed: true", transaction.toString());
+                + "comitted: true, rolledBack: false", transaction.toString());
         context.assertIsSatisfied();
     }
 
@@ -601,7 +586,7 @@ public class MultiDataSetArchiverTest extends AbstractFileSystemTestCase
         assertEquals("[ds2]: AVAILABLE true\n", statusUpdater.toString());
         assertEquals("Containers:\nMultiDataSetArchiverContainerDTO [id=0, path=ds2.tar]\n"
                 + "Data sets:\nMultiDataSetArchiverDataSetDTO [id=1, code=ds2, containerId=0, sizeInBytes=20]\n"
-                + "comitted: true, rolledBack: false, closed: true", transaction.toString());
+                + "comitted: true, rolledBack: false", transaction.toString());
         context.assertIsSatisfied();
     }