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 98c695ecf35b0aea04f003802723759b46806e51..4bb8661d9f6e5a98b196f1c35887724d4b0b5720 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 38e9b7d0e110648558243d37979bbbb79664a54f..ebc4cc7f81f7e4d2b02c96b97d6477e07bda8e2f 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 ff2c02d763105f668f0ee5073172d64df452a186..ea2bf487edfd45088b55b76cf1e27fd2b1996720 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 18e9c620d738596959515f05d631501d728b1589..45fa1be56b900aa1c5e7c1978fc8bb9e8f8705ed 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 3aea15764840731779d87dea66adac9d73e8cdc1..f0e393550c44fe89cd241dc1bfaaea46a78359d2 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 ce44efe248f3664e4e218e8d29e0aef3bbaeefce..bc545a1e5945a0060e02c2f4079fca16100bce9f 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 b953ab667ed39df7b4a55bd9cb7d3868a473e2eb..3c6b0e7f62aeea33a69cc7273a34e1b74a215fce 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 3bb5a25881d002e04b8215a5e00a9a91df1ba5a8..2957285e084119cda00bddc0485dd326df5028c5 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 66a794e7dd88bb7224246b524c367b03f090e5e4..d303760860e202fcb907bd11bc7f60ff36c8dd6a 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);