From 8c32d844abe6f813358a44b30978d49e1ae5db78 Mon Sep 17 00:00:00 2001
From: felmer <felmer>
Date: Wed, 14 May 2014 13:56:08 +0000
Subject: [PATCH] SSDM-121: Data Set Metadata Update allows more than one
 container.

SVN: 31506
---
 .../resultset/TableForUpdateExporter.java     | 14 ++-
 .../bo/AbstractDataSetBusinessObject.java     | 43 +++++++---
 .../generic/server/business/bo/DataBO.java    |  2 +-
 .../server/business/bo/DataSetTable.java      |  2 +-
 .../server/business/bo/DataBOTest.java        | 86 +++++++++++++++++--
 5 files changed, 120 insertions(+), 27 deletions(-)

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/TableForUpdateExporter.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/TableForUpdateExporter.java
index 3dadaff8232..88a4e8326fd 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/TableForUpdateExporter.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/server/resultset/TableForUpdateExporter.java
@@ -27,17 +27,18 @@ import java.util.Map.Entry;
 import java.util.Set;
 import java.util.TreeMap;
 
+import ch.systemsx.cisd.common.shared.basic.string.CommaSeparatedListBuilder;
 import ch.systemsx.cisd.openbis.generic.client.web.client.dto.GridRowModels;
 import ch.systemsx.cisd.openbis.generic.client.web.server.IBasicTableDataProvider;
 import ch.systemsx.cisd.openbis.generic.client.web.server.util.TSVRenderer;
 import ch.systemsx.cisd.openbis.generic.shared.ICommonServer;
 import ch.systemsx.cisd.openbis.generic.shared.basic.GridRowModel;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AbstractExternalData;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ContainerDataSet;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.EntityKind;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.EntityType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.EntityTypePropertyType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AbstractExternalData;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.IEntityProperty;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewDataSet;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewExperiment;
@@ -256,10 +257,15 @@ public class TableForUpdateExporter
             AbstractExternalData dataSet = row.getOriginalObject().getObjectOrNull();
             builder.addRow(dataSet);
             builder.column(NewDataSet.CODE).addString(dataSet.getCode());
-            ContainerDataSet container = dataSet.tryGetContainer();
-            if (container != null)
+            List<ContainerDataSet> containerDataSets = dataSet.getContainerDataSets();
+            if (containerDataSets.isEmpty() == false)
             {
-                builder.column(NewDataSet.CONTAINER).addString(container.getCode());
+                CommaSeparatedListBuilder listBuilder = new CommaSeparatedListBuilder();
+                for (ContainerDataSet containerDataSet : containerDataSets)
+                {
+                    listBuilder.append(containerDataSet.getCode());
+                }
+                builder.column(NewDataSet.CONTAINER).addString(listBuilder.toString());
             }
             Collection<AbstractExternalData> parents = dataSet.getParents();
             if (parents != null && parents.isEmpty() == false)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractDataSetBusinessObject.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractDataSetBusinessObject.java
index 66209c672a5..a12ae2d5cfe 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractDataSetBusinessObject.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractDataSetBusinessObject.java
@@ -17,10 +17,13 @@
 package ch.systemsx.cisd.openbis.generic.server.business.bo;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import ch.systemsx.cisd.common.collection.CollectionUtils;
@@ -345,26 +348,38 @@ public abstract class AbstractDataSetBusinessObject extends AbstractSampleIdenti
         }
     }
 
-    protected void updateContainer(DataPE data, String containerCode)
+    protected void updateContainers(DataPE data, String containerCodes)
     {
-        if (containerCode != null)
+        if (containerCodes == null)
         {
-            DataPE container = getDataDAO().tryToFindDataSetByCode(containerCode);
-
-            if (container == null)
-            {
-                throw UserFailureException.fromTemplate(
-                        "Data Set with a following code doesn't exist: '%s'.", containerCode);
-            }
-            if (!container.isContainer())
+            return;
+        }
+        List<String> codes = Arrays.asList(containerCodes.split(" *, *"));
+        Map<String, DataPE> newContainers = getDataSets(codes);
+        List<DataPE> oldContainers = data.getContainers();
+        for (DataPE oldContainer : oldContainers)
+        {
+            if (newContainers.remove(oldContainer.getCode()) == null)
             {
-                throw UserFailureException.fromTemplate(
-                        "Data Set with a following code is not a container: '%s'.", containerCode);
+                relationshipService.removeDataSetFromContainer(session, data, oldContainer);
             }
+        }
+        for (DataPE newContainer : newContainers.values())
+        {
+            relationshipService.assignDataSetToContainer(session, data, newContainer);
+            validateContainerContainedRelationshipGraph(newContainer, data);
+        }
+    }
 
-            validateContainerContainedRelationshipGraph(container, data);
-            relationshipService.assignDataSetToContainer(session, data, container);
+    private Map<String, DataPE> getDataSets(List<String> codes)
+    {
+        Set<DataPE> dataSets = findDataSetsByCodes(codes);
+        Map<String, DataPE> dataSetsByCode = new HashMap<String, DataPE>();
+        for (DataPE dataSet : dataSets)
+        {
+            dataSetsByCode.put(dataSet.getCode(), dataSet);
         }
+        return dataSetsByCode;
     }
 
     private void validateContainerContainedRelationshipGraph(DataPE container, DataPE contained)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataBO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataBO.java
index 29e4708565e..b11d8acd4f4 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataBO.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataBO.java
@@ -652,7 +652,7 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO
             return; // container data set was not changed
         } else
         {
-            updateContainer(data, modifiedContainerDatasetCodeOrNull);
+            updateContainers(data, modifiedContainerDatasetCodeOrNull);
         }
     }
 
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataSetTable.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataSetTable.java
index 626174af647..2e0c4a7d65e 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataSetTable.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataSetTable.java
@@ -1098,7 +1098,7 @@ public final class DataSetTable extends AbstractDataSetBusinessObject implements
 
             if (dataSetUpdates.getModifiedContainerDatasetCodeOrNull() != null)
             {
-                updateContainer(dataSet, dataSetUpdates.getModifiedContainerDatasetCodeOrNull());
+                updateContainers(dataSet, dataSetUpdates.getModifiedContainerDatasetCodeOrNull());
             }
         }
         if (details.isParentsUpdateRequested())
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataBOTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataBOTest.java
index 0f0ae4a6491..6fe2466acda 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataBOTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataBOTest.java
@@ -25,6 +25,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 
 import org.hamcrest.BaseMatcher;
@@ -734,6 +735,52 @@ public class DataBOTest extends AbstractBOTest
         context.assertIsSatisfied();
     }
 
+    @Test
+    public void testUpdateChangeContainers()
+    {
+        ExperimentPE experiment = new ExperimentPE();
+        experiment.setCode(EXPERIMENT_IDENTIFIER.getExperimentCode());
+        experiment.setProject(ManagerTestTool.EXAMPLE_PROJECT);
+        final DataPE ds1 = createDataSet("ds-1", null, experiment);
+        DataPE ds2 = createDataSet("cds-2", null, experiment);
+        DataPE ds3 = createDataSet("cds-3", null, experiment);
+        DataPE ds4 = createDataSet("cds-4", null, experiment);
+        DataPE ds5 = createDataSet("cds-5", null, experiment);
+        RelationshipTypePE relationshipType = new RelationshipTypePE();
+        relationshipType.setCode(BasicConstant.CONTAINER_COMPONENT_INTERNAL_RELATIONSHIP);
+        ds1.setParentRelationships(new LinkedHashSet<DataSetRelationshipPE>(Arrays.asList(new DataSetRelationshipPE(ds2, ds1,
+                relationshipType, 1, null))));
+        DataSetUpdatesDTO dataSetUpdatesDTO = createDataSetUpdates(ds1, null, EXPERIMENT_IDENTIFIER);
+        dataSetUpdatesDTO.setModifiedContainerDatasetCodeOrNull(ds3.getCode() + "," + ds4.getCode() + "  ,   " + ds5.getCode());
+        prepareTryToFindDataSetByCode(ds3);
+        prepareTryToFindDataSetByCode(ds4);
+        prepareTryToFindDataSetByCode(ds5);
+        prepareForUpdate(ds1, experiment);
+        prepareRemoveDataSetFromContainer(ds1, ds2);
+        prepareAssignDataSetToContainer(ds1, ds3);
+        prepareAssignDataSetToContainer(ds1, ds4);
+        prepareAssignDataSetToContainer(ds1, ds5);
+        context.checking(new Expectations()
+            {
+                {
+                    one(propertiesConverter).updateProperties(
+                            ds1.getProperties(), ds1.getDataSetType(),
+                            null, ManagerTestTool.EXAMPLE_PERSON, Collections.<String> emptySet());
+                    will(returnValue(Collections.emptySet()));
+
+                    expectMandatoryPropertiesCheck(this, ds1.getDataSetType());
+
+                    one(dataDAO).validateAndSaveUpdatedEntity(ds1);
+                }
+            });
+
+        IDataBO dataBO = createDataBO();
+
+        dataBO.update(dataSetUpdatesDTO);
+
+        context.assertIsSatisfied();
+    }
+
     @Test
     public void testUpdateWithDataSetAsItsOwnContainer()
     {
@@ -794,13 +841,7 @@ public class DataBOTest extends AbstractBOTest
         dataSetUpdatesDTO.setModifiedContainedDatasetCodesOrNull(componentCodes);
 
         prepareForUpdate(container1, experiment);
-        context.checking(new Expectations()
-            {
-                {
-                    one(dataDAO).tryToFindDataSetByCode(container2.getCode());
-                    will(returnValue(container2));
-                }
-            });
+        prepareTryToFindDataSetByCode(container2);
 
         IDataBO dataBO = createDataBO();
         try
@@ -815,6 +856,37 @@ public class DataBOTest extends AbstractBOTest
         context.assertIsSatisfied();
     }
 
+    private void prepareRemoveDataSetFromContainer(final DataPE dataSet, final DataPE container)
+    {
+        context.checking(new Expectations()
+            {
+                {
+                    one(relationshipService).removeDataSetFromContainer(EXAMPLE_SESSION, dataSet, container);
+                }
+            });
+    }
+
+    private void prepareAssignDataSetToContainer(final DataPE dataSet, final DataPE container)
+    {
+        context.checking(new Expectations()
+            {
+                {
+                    one(relationshipService).assignDataSetToContainer(EXAMPLE_SESSION, dataSet, container);
+                }
+            });
+    }
+
+    private void prepareTryToFindDataSetByCode(final DataPE dataSet)
+    {
+        context.checking(new Expectations()
+            {
+                {
+                    allowing(dataDAO).tryToFindDataSetByCode(dataSet.getCode());
+                    will(returnValue(dataSet));
+                }
+            });
+    }
+
     private void prepareForUpdate(final ExternalDataPE dataSet, final SamplePE sample)
     {
         context.checking(new Expectations()
-- 
GitLab