From 2c90904c2c06ddb8753e2d7a10f924c939cf21c9 Mon Sep 17 00:00:00 2001
From: pkupczyk <pkupczyk>
Date: Thu, 5 Nov 2015 12:18:37 +0000
Subject: [PATCH] SSDM-2706 : V3 AS API - create data sets (only metadata) -
 check that container is actually of container type (also during data set
 update)

SVN: 34997
---
 .../UpdateDataSetChildrenExecutor.java        |  2 +-
 .../UpdateDataSetContainedExecutor.java       |  8 +-
 .../UpdateDataSetContainersExecutor.java      |  8 +-
 .../dataset/UpdateDataSetParentsExecutor.java |  2 +-
 ...actUpdateEntityToManyRelationExecutor.java |  6 +-
 .../sample/UpdateSampleChildrenExecutor.java  |  2 +-
 .../sample/UpdateSampleContainedExecutor.java |  2 +-
 .../sample/UpdateSampleParentsExecutor.java   |  2 +-
 .../systemtest/api/v3/UpdateDataSetTest.java  | 93 +++++++++++--------
 9 files changed, 78 insertions(+), 47 deletions(-)

diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetChildrenExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetChildrenExecutor.java
index 98c695ecf35..4bb8661d9f6 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetChildrenExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetChildrenExecutor.java
@@ -50,7 +50,7 @@ public class UpdateDataSetChildrenExecutor extends AbstractUpdateEntityToManyRel
     }
 
     @Override
-    protected void check(IOperationContext context, IDataSetId relatedId, DataPE related)
+    protected void check(IOperationContext context, DataPE entity, IDataSetId relatedId, DataPE related)
     {
         if (false == new DataSetPEByExperimentOrSampleIdentifierValidator().doValidation(context.getSession().tryGetPerson(), related))
         {
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetContainedExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetContainedExecutor.java
index 38e9b7d0e11..ebc4cc7f81f 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetContainedExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetContainedExecutor.java
@@ -26,6 +26,7 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.IdListUpdateValue;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.dataset.DataSetUpdate;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.dataset.IDataSetId;
 import ch.ethz.sis.openbis.generic.shared.api.v3.exceptions.UnauthorizedObjectAccessException;
+import ch.systemsx.cisd.common.exceptions.UserFailureException;
 import ch.systemsx.cisd.openbis.generic.server.authorization.validator.DataSetPEByExperimentOrSampleIdentifierValidator;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE;
 
@@ -50,12 +51,17 @@ public class UpdateDataSetContainedExecutor extends AbstractUpdateEntityToManyRe
     }
 
     @Override
-    protected void check(IOperationContext context, IDataSetId relatedId, DataPE related)
+    protected void check(IOperationContext context, DataPE entity, IDataSetId relatedId, DataPE related)
     {
         if (false == new DataSetPEByExperimentOrSampleIdentifierValidator().doValidation(context.getSession().tryGetPerson(), related))
         {
             throw new UnauthorizedObjectAccessException(relatedId);
         }
+        if (false == entity.isContainer())
+        {
+            throw new UserFailureException("Data set " + entity.getCode()
+                    + " is not of a container type therefore cannot have contained data sets.");
+        }
     }
 
     @Override
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetContainersExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetContainersExecutor.java
index ff2c02d7631..ea2bf487edf 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetContainersExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetContainersExecutor.java
@@ -26,6 +26,7 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.IdListUpdateValue;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.dataset.DataSetUpdate;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.dataset.IDataSetId;
 import ch.ethz.sis.openbis.generic.shared.api.v3.exceptions.UnauthorizedObjectAccessException;
+import ch.systemsx.cisd.common.exceptions.UserFailureException;
 import ch.systemsx.cisd.openbis.generic.server.authorization.validator.DataSetPEByExperimentOrSampleIdentifierValidator;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE;
 
@@ -50,7 +51,7 @@ public class UpdateDataSetContainersExecutor extends AbstractUpdateEntityToManyR
     }
 
     @Override
-    protected void check(IOperationContext context, IDataSetId relatedId, DataPE related)
+    protected void check(IOperationContext context, DataPE entity, IDataSetId relatedId, DataPE related)
     {
         if (false == new DataSetPEByExperimentOrSampleIdentifierValidator().doValidation(context.getSession().tryGetPerson(), related))
         {
@@ -61,6 +62,11 @@ public class UpdateDataSetContainersExecutor extends AbstractUpdateEntityToManyR
     @Override
     protected void add(IOperationContext context, DataPE entity, DataPE related)
     {
+        if (false == related.isContainer())
+        {
+            throw new UserFailureException("Data set " + related.getCode()
+                    + " is not of a container type therefore cannot be set as a container of data set " + entity.getCode() + ".");
+        }
         relationshipService.assignDataSetToContainer(context.getSession(), entity, related);
     }
 
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetParentsExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetParentsExecutor.java
index 18e9c620d73..45fa1be56b9 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetParentsExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetParentsExecutor.java
@@ -50,7 +50,7 @@ public class UpdateDataSetParentsExecutor extends AbstractUpdateEntityToManyRela
     }
 
     @Override
-    protected void check(IOperationContext context, IDataSetId relatedId, DataPE related)
+    protected void check(IOperationContext context, DataPE entity, IDataSetId relatedId, DataPE related)
     {
         if (false == new DataSetPEByExperimentOrSampleIdentifierValidator().doValidation(context.getSession().tryGetPerson(), related))
         {
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/entity/AbstractUpdateEntityToManyRelationExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/entity/AbstractUpdateEntityToManyRelationExecutor.java
index 3aea1576484..f0e393550c4 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/entity/AbstractUpdateEntityToManyRelationExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/entity/AbstractUpdateEntityToManyRelationExecutor.java
@@ -69,7 +69,7 @@ public abstract class AbstractUpdateEntityToManyRelationExecutor<ENTITY_UPDATE,
                             {
                                 throw new ObjectNotFoundException((IObjectId) relatedId);
                             }
-                            check(context, relatedId, related);
+                            check(context, entity, relatedId, related);
                             relatedCollection.add(related);
                         }
                         if (action instanceof ListUpdateActionSet<?>)
@@ -88,7 +88,7 @@ public abstract class AbstractUpdateEntityToManyRelationExecutor<ENTITY_UPDATE,
                             {
                                 relatedCollection.add(related);
                             }
-                            check(context, relatedId, related);
+                            check(context, entity, relatedId, related);
                         }
                         remove(context, entity, relatedCollection);
                     }
@@ -150,7 +150,7 @@ public abstract class AbstractUpdateEntityToManyRelationExecutor<ENTITY_UPDATE,
 
     protected abstract IdListUpdateValue<? extends RELATED_ID> getRelatedUpdate(IOperationContext context, ENTITY_UPDATE update);
 
-    protected abstract void check(IOperationContext context, RELATED_ID relatedId, RELATED_PE related);
+    protected abstract void check(IOperationContext context, ENTITY_PE entity, RELATED_ID relatedId, RELATED_PE related);
 
     protected abstract void add(IOperationContext context, ENTITY_PE entity, RELATED_PE related);
 
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleChildrenExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleChildrenExecutor.java
index ce44efe248f..bc545a1e594 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleChildrenExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleChildrenExecutor.java
@@ -50,7 +50,7 @@ public class UpdateSampleChildrenExecutor extends AbstractUpdateEntityToManyRela
     }
 
     @Override
-    protected void check(IOperationContext context, ISampleId relatedId, SamplePE related)
+    protected void check(IOperationContext context, SamplePE entity, ISampleId relatedId, SamplePE related)
     {
         if (false == new SampleByIdentiferValidator().doValidation(context.getSession().tryGetPerson(), related))
         {
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleContainedExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleContainedExecutor.java
index b953ab667ed..3c6b0e7f62a 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleContainedExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleContainedExecutor.java
@@ -54,7 +54,7 @@ public class UpdateSampleContainedExecutor extends AbstractUpdateEntityToManyRel
     }
 
     @Override
-    protected void check(IOperationContext context, ISampleId relatedId, SamplePE related)
+    protected void check(IOperationContext context, SamplePE entity, ISampleId relatedId, SamplePE related)
     {
         if (false == new SampleByIdentiferValidator().doValidation(context.getSession().tryGetPerson(), related))
         {
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleParentsExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleParentsExecutor.java
index 3bb5a25881d..2957285e084 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleParentsExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/UpdateSampleParentsExecutor.java
@@ -50,7 +50,7 @@ public class UpdateSampleParentsExecutor extends AbstractUpdateEntityToManyRelat
     }
 
     @Override
-    protected void check(IOperationContext context, ISampleId relatedId, SamplePE related)
+    protected void check(IOperationContext context, SamplePE entity, ISampleId relatedId, SamplePE related)
     {
         if (false == new SampleByIdentiferValidator().doValidation(context.getSession().tryGetPerson(), related))
         {
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/UpdateDataSetTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/UpdateDataSetTest.java
index 66a794e7dd8..d303760860e 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/UpdateDataSetTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/UpdateDataSetTest.java
@@ -27,9 +27,6 @@ import java.util.Map;
 
 import org.testng.annotations.Test;
 
-import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.ListUpdateValue.ListUpdateAction;
-import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.ListUpdateValue.ListUpdateActionAdd;
-import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.ListUpdateValue.ListUpdateActionRemove;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.dataset.DataSet;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.dataset.DataSetUpdate;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.dataset.PhysicalDataUpdate;
@@ -42,7 +39,6 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.experiment.ExperimentIde
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.experiment.ExperimentPermId;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.experiment.IExperimentId;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.sample.SamplePermId;
-import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.tag.ITagId;
 import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.tag.TagCode;
 import ch.systemsx.cisd.common.action.IDelegatedAction;
 import ch.systemsx.cisd.common.exceptions.UserFailureException;
@@ -399,10 +395,7 @@ public class UpdateDataSetTest extends AbstractSampleTest
 
         DataSetUpdate update = new DataSetUpdate();
         update.setDataSetId(dataSetId);
-        ListUpdateAction<IDataSetId> addAction = new ListUpdateActionAdd<IDataSetId>();
-        addAction.setItems(Collections.singletonList(new DataSetPermId("20081105092259000-20")));
-
-        update.setParentActions(Collections.singletonList(addAction));
+        update.getParentIds().add(new DataSetPermId("20081105092259000-20"));
 
         v3api.updateDataSets(sessionToken, Collections.singletonList(update));
 
@@ -423,9 +416,7 @@ public class UpdateDataSetTest extends AbstractSampleTest
 
         DataSetUpdate update = new DataSetUpdate();
         update.setDataSetId(originalChild);
-        ListUpdateAction<IDataSetId> removeAction = new ListUpdateActionRemove<IDataSetId>();
-        removeAction.setItems(Collections.singletonList(originalParent));
-        update.setParentActions(Collections.singletonList(removeAction));
+        update.getParentIds().remove(originalParent);
 
         assertRemovingParent(originalChild, originalParent, update);
     }
@@ -460,9 +451,7 @@ public class UpdateDataSetTest extends AbstractSampleTest
 
         DataSetUpdate update = new DataSetUpdate();
         update.setDataSetId(originalParent);
-        ListUpdateAction<IDataSetId> removeAction = new ListUpdateActionRemove<IDataSetId>();
-        removeAction.setItems(Collections.singletonList(originalChild));
-        update.setChildActions(Collections.singletonList(removeAction));
+        update.getChildIds().remove(originalChild);
 
         assertRemovingParent(originalChild, originalParent, update);
     }
@@ -494,8 +483,8 @@ public class UpdateDataSetTest extends AbstractSampleTest
     {
         final String sessionToken = v3api.login(TEST_USER, PASSWORD);
 
-        DataSetPermId dataSetId = new DataSetPermId("20081105092259000-8");
-        final DataSetPermId componentId = new DataSetPermId("20081105092259000-9");
+        DataSetPermId dataSetId = new DataSetPermId("CONTAINER_1");
+        final DataSetPermId componentId = new DataSetPermId("CONTAINER_2");
 
         final DataSetUpdate update = new DataSetUpdate();
         update.setDataSetId(dataSetId);
@@ -509,7 +498,7 @@ public class UpdateDataSetTest extends AbstractSampleTest
                 {
                     v3api.updateDataSets(sessionToken, Collections.singletonList(update));
                 }
-            }, "Circular dependency found: 20081105092259000-8");
+            }, "Circular dependency found: CONTAINER_1");
     }
 
     @Test
@@ -521,12 +510,8 @@ public class UpdateDataSetTest extends AbstractSampleTest
 
         DataSetUpdate update = new DataSetUpdate();
         update.setDataSetId(dataSetId);
-        ListUpdateActionAdd<IDataSetId> addAction = new ListUpdateActionAdd<IDataSetId>();
-        ListUpdateAction<IDataSetId> removeAction = new ListUpdateActionRemove<IDataSetId>();
-        addAction.setItems(Collections.singletonList(new DataSetPermId("COMPONENT_2A")));
-        removeAction.setItems(Collections.singletonList(new DataSetPermId("COMPONENT_1A")));
-
-        update.setContainedActions(Arrays.asList(addAction, removeAction));
+        update.getContainedIds().add(new DataSetPermId("COMPONENT_2A"));
+        update.getContainedIds().remove(new DataSetPermId("COMPONENT_1A"));
 
         v3api.updateDataSets(sessionToken, Collections.singletonList(update));
 
@@ -537,6 +522,25 @@ public class UpdateDataSetTest extends AbstractSampleTest
         AssertionUtil.assertCollectionContainsOnly(dataSetCodes(result.getContained()), "COMPONENT_1B", "COMPONENT_2A");
     }
 
+    @Test
+    public void testUpdateWithComponentWithNonContainerDataSet()
+    {
+        final String sessionToken = v3api.login(TEST_USER, PASSWORD);
+
+        final DataSetUpdate update = new DataSetUpdate();
+        update.setDataSetId(new DataSetPermId("COMPONENT_1A"));
+        update.getContainedIds().add(new DataSetPermId("COMPONENT_2A"));
+
+        assertUserFailureException(new IDelegatedAction()
+            {
+                @Override
+                public void execute()
+                {
+                    v3api.updateDataSets(sessionToken, Collections.singletonList(update));
+                }
+            }, "Data set COMPONENT_1A is not of a container type therefore cannot have contained data sets.");
+    }
+
     @Test
     public void testUpdateWithContainerAddRemove()
     {
@@ -550,12 +554,8 @@ public class UpdateDataSetTest extends AbstractSampleTest
 
         DataSetUpdate update = new DataSetUpdate();
         update.setDataSetId(dataSetId);
-        ListUpdateActionAdd<IDataSetId> addAction = new ListUpdateActionAdd<IDataSetId>();
-        ListUpdateAction<IDataSetId> removeAction = new ListUpdateActionRemove<IDataSetId>();
-        addAction.setItems(Arrays.asList(cont2, cont3a));
-        removeAction.setItems(Collections.singletonList(cont1));
-
-        update.setContainerActions(Arrays.asList(addAction, removeAction));
+        update.getContainerIds().add(cont2, cont3a);
+        update.getContainerIds().remove(cont1);
 
         v3api.updateDataSets(sessionToken, Collections.singletonList(update));
 
@@ -572,6 +572,28 @@ public class UpdateDataSetTest extends AbstractSampleTest
         AssertionUtil.assertCollectionContains(dataSetCodes(map.get(cont3a).getContained()), dataSetId.getPermId());
     }
 
+    @Test
+    public void testUpdateWithContainerAddNotContainerDataSet()
+    {
+        final String sessionToken = v3api.login(TEST_USER, PASSWORD);
+
+        DataSetPermId dataSetId = new DataSetPermId("CONTAINER_1");
+        DataSetPermId containerId = new DataSetPermId("COMPONENT_2A");
+
+        final DataSetUpdate update = new DataSetUpdate();
+        update.setDataSetId(dataSetId);
+        update.getContainerIds().add(containerId);
+
+        assertUserFailureException(new IDelegatedAction()
+            {
+                @Override
+                public void execute()
+                {
+                    v3api.updateDataSets(sessionToken, Collections.singletonList(update));
+                }
+            }, "Data set COMPONENT_2A is not of a container type therefore cannot be set as a container of data set CONTAINER_1.");
+    }
+
     @Test
     public void testUpdateWithContainerUnauthorized()
     {
@@ -610,9 +632,8 @@ public class UpdateDataSetTest extends AbstractSampleTest
         // add tag1
         DataSetUpdate firstUpdate = new DataSetUpdate();
         firstUpdate.setDataSetId(dataSet);
-        ListUpdateAction<ITagId> addAction = new ListUpdateActionAdd<ITagId>();
-        addAction.setItems(Arrays.asList(newTag1));
-        firstUpdate.setTagActions(Arrays.asList(addAction));
+        firstUpdate.getTagIds().add(newTag1);
+
         v3api.updateDataSets(sessionToken, Arrays.asList(firstUpdate));
 
         Map<IDataSetId, DataSet> result = v3api.mapDataSets(sessionToken, Arrays.asList(dataSet), fe);
@@ -621,11 +642,9 @@ public class UpdateDataSetTest extends AbstractSampleTest
         // remove test_metaprojects and add a new tag2
         DataSetUpdate secondUpdate = new DataSetUpdate();
         secondUpdate.setDataSetId(dataSet);
-        addAction = new ListUpdateActionAdd<ITagId>();
-        ListUpdateAction<ITagId> removeAction = new ListUpdateActionRemove<ITagId>();
-        addAction.setItems(Arrays.asList(newTag2));
-        removeAction.setItems(Arrays.asList(existingTag));
-        secondUpdate.setTagActions(Arrays.asList(addAction, removeAction));
+        secondUpdate.getTagIds().remove(existingTag);
+        secondUpdate.getTagIds().add(newTag2);
+
         v3api.updateDataSets(sessionToken, Arrays.asList(secondUpdate));
 
         result = v3api.mapDataSets(sessionToken, Arrays.asList(dataSet), fe);
-- 
GitLab