From 953269ea85d16a2ba8b2be6714206016d136ae93 Mon Sep 17 00:00:00 2001 From: felmer <felmer> Date: Tue, 4 Nov 2014 10:40:55 +0000 Subject: [PATCH] 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 --- .../archiver/MultiDataSetArchiver.java | 22 +++++++++----- .../IMultiDataSetArchiverDBTransaction.java | 11 ------- .../MultiDataSetArchiverDBTransaction.java | 9 ------ .../archiver/MultiDataSetArchiverTest.java | 29 +++++-------------- 4 files changed, 21 insertions(+), 50 deletions(-) diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/MultiDataSetArchiver.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/MultiDataSetArchiver.java index 54efc79116a..47953e9bae9 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/MultiDataSetArchiver.java +++ b/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; } diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/dataaccess/IMultiDataSetArchiverDBTransaction.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/dataaccess/IMultiDataSetArchiverDBTransaction.java index 82793548800..d879833bd9f 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/dataaccess/IMultiDataSetArchiverDBTransaction.java +++ b/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 diff --git a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/dataaccess/MultiDataSetArchiverDBTransaction.java b/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/dataaccess/MultiDataSetArchiverDBTransaction.java index d3bc0726001..dd3ec72f0d6 100644 --- a/datastore_server/source/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/dataaccess/MultiDataSetArchiverDBTransaction.java +++ b/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(); - } - } diff --git a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/MultiDataSetArchiverTest.java b/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/MultiDataSetArchiverTest.java index 4daa97757a4..f557f74eadc 100644 --- a/datastore_server/sourceTest/java/ch/systemsx/cisd/openbis/dss/generic/server/plugins/standard/archiver/MultiDataSetArchiverTest.java +++ b/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(); } -- GitLab