diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetExperimentExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetExperimentExecutor.java index bca4ba5c6d08d72e6538ff298e03aa7d0568d27e..18a714740b638c0c289d9e6a07bbf4bdd85a1c91 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetExperimentExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetExperimentExecutor.java @@ -30,6 +30,7 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.dataset.DataSetUpdat import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.experiment.ExperimentIdentifier; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.experiment.IExperimentId; 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.ExperimentByIdentiferValidator; import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE; import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE; @@ -82,7 +83,13 @@ public class UpdateDataSetExperimentExecutor extends @Override protected void update(IOperationContext context, DataPE entity, ExperimentPE related) { - relationshipService.assignDataSetToExperiment(context.getSession(), entity, related); + if (related == null) + { + throw new UserFailureException("Data set '" + entity.getCode() + "' cannot have experiment set to null"); + } else + { + relationshipService.assignDataSetToExperiment(context.getSession(), entity, related); + } } } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/VerifyDataSetExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/VerifyDataSetExecutor.java index e1927793d7c067b8cc8b3977c0b0346ab1286a68..214acaeb7ffafac0414845f7d0e78497b9bc69ae 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/VerifyDataSetExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/VerifyDataSetExecutor.java @@ -44,14 +44,6 @@ public class VerifyDataSetExecutor implements IVerifyDataSetExecutor @Override public void verify(IOperationContext context, Collection<DataPE> dataSets) { - for (DataPE dataSet : dataSets) - { - if (dataSet.getExperiment() == null && dataSet.tryGetSample() == null) - { - throw new IllegalArgumentException("Data set '" + dataSet.getCode() + "' has both experiment and sample set to null"); - } - } - verifyEntityPropertyExecutor.verify(context, dataSets); verifyDataSetContainersExecutor.verify(context, dataSets); verifyDataSetParentsExecutor.verify(context, dataSets); diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/dataset/DataSetTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/dataset/DataSetTranslator.java index 06d8bc67b9e83658a1bdd30c3081d5f7f04de934..dfc71830e2d70738aaeb3ba44a7d04c9d0caf637 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/dataset/DataSetTranslator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/dataset/DataSetTranslator.java @@ -130,12 +130,15 @@ public class DataSetTranslator extends AbstractCachingTranslator<DataPE, DataSet result.getFetchOptions().withExperimentUsing(getFetchOptions().withExperiment()); } - if (getFetchOptions().hasSample() && dataPe.tryGetSample() != null) + if (getFetchOptions().hasSample()) { - Sample sample = - new SampleTranslator(getTranslationContext(), managedPropertyEvaluatorFactory, getFetchOptions().withSample()) - .translate(dataPe.tryGetSample()); - result.setSample(sample); + if (dataPe.tryGetSample() != null) + { + Sample sample = + new SampleTranslator(getTranslationContext(), managedPropertyEvaluatorFactory, getFetchOptions().withSample()) + .translate(dataPe.tryGetSample()); + result.setSample(sample); + } result.getFetchOptions().withSampleUsing(getFetchOptions().withSample()); } 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 aced200e6428ed382fd788e8c8e2e7d58cc15d6b..7d7ade1d216246864aabb9858ad75a5372b353de 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 @@ -17,6 +17,7 @@ package ch.ethz.sis.openbis.systemtest.api.v3; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; import java.util.Arrays; import java.util.Collection; @@ -37,7 +38,9 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.dataset.DataSe import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.dataset.DataSetPermId; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.dataset.FileFormatTypePermId; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.dataset.IDataSetId; +import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.experiment.ExperimentIdentifier; 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; @@ -149,6 +152,35 @@ public class UpdateDataSetTest extends AbstractSampleTest }, dataSetId); } + @Test + public void testUpdateWithSampleNull() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20081105092159111-1"); + DataSetFetchOptions fe = new DataSetFetchOptions(); + fe.withExperiment(); + fe.withSample(); + + Map<IDataSetId, DataSet> map = v3api.mapDataSets(sessionToken, Arrays.asList(dataSetId), fe); + + DataSet result = map.get(dataSetId); + assertEquals(result.getExperiment().getPermId().getPermId(), "200902091239077-1033"); + assertEquals(result.getSample().getPermId().getPermId(), "200902091219327-1025"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.setSampleId(null); + + v3api.updateDataSets(sessionToken, Arrays.asList(update)); + + map = v3api.mapDataSets(sessionToken, Arrays.asList(dataSetId), fe); + + result = map.get(dataSetId); + assertEquals(result.getExperiment().getPermId().getPermId(), "200902091239077-1033"); + assertNull(result.getSample()); + } + @Test public void testUpdateWithExperiment() { @@ -170,6 +202,79 @@ public class UpdateDataSetTest extends AbstractSampleTest assertEquals(result.getExperiment().getPermId().getPermId(), "200811050951882-1028"); } + @Test + public void testUpdateWithExperimentUnauthorized() + { + final String sessionToken = v3api.login(TEST_SPACE_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20120619092259000-22"); + IExperimentId experimentId = new ExperimentIdentifier("/CISD/NEMO/EXP1"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.setExperimentId(experimentId); + + assertUnauthorizedObjectAccessException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + } + }, experimentId); + } + + @Test + public void testUpdateWithExperimentNull() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20081105092259000-18"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.setExperimentId(null); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + } + }, "Data set '20081105092259000-18' cannot have experiment set to null"); + } + + @Test + public void testUpdateWithExperimentAndSample() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20081105092159111-1"); + DataSetFetchOptions fe = new DataSetFetchOptions(); + fe.withExperiment(); + fe.withSample(); + + Map<IDataSetId, DataSet> map = v3api.mapDataSets(sessionToken, Arrays.asList(dataSetId), fe); + + DataSet result = map.get(dataSetId); + assertEquals(result.getExperiment().getPermId().getPermId(), "200902091239077-1033"); + assertEquals(result.getSample().getPermId().getPermId(), "200902091219327-1025"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.setExperimentId(new ExperimentPermId("200902091258949-1034")); + update.setSampleId(new SamplePermId("200902091250077-1026")); + + v3api.updateDataSets(sessionToken, Arrays.asList(update)); + + map = v3api.mapDataSets(sessionToken, Arrays.asList(dataSetId), fe); + + result = map.get(dataSetId); + assertEquals(result.getExperiment().getPermId().getPermId(), "200902091258949-1034"); + assertEquals(result.getSample().getPermId().getPermId(), "200902091250077-1026"); + } + @Test public void testUpdateWithExternalDataSet() { @@ -193,6 +298,51 @@ public class UpdateDataSetTest extends AbstractSampleTest assertEquals(result.getExternalData().getFileFormatType().getCode(), "PLKPROPRIETARY"); } + @Test + public void testUpdateWithParentUnauthorized() + { + final String sessionToken = v3api.login(TEST_SPACE_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20120619092259000-22"); + final DataSetPermId parentId = new DataSetPermId("20081105092159111-1"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.getParentIds().add(parentId); + + assertUnauthorizedObjectAccessException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + } + }, parentId); + } + + @Test + public void testUpdateWithParentCircularDependency() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20081105092259000-8"); + final DataSetPermId parentId = new DataSetPermId("20081105092259000-9"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.getParentIds().add(parentId); + update.getChildIds().add(parentId); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + } + }, "Circular dependency found: 20081105092259000-8"); + } + @Test public void testUpdateWithParentAdd() { @@ -233,6 +383,28 @@ public class UpdateDataSetTest extends AbstractSampleTest assertRemovingParent(originalChild, originalParent, update); } + @Test + public void testUpdateWithChildrenUnauthorized() + { + final String sessionToken = v3api.login(TEST_SPACE_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20120619092259000-22"); + final DataSetPermId childId = new DataSetPermId("20081105092159111-1"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.getChildIds().add(childId); + + assertUnauthorizedObjectAccessException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + } + }, childId); + } + @Test public void testUpdateWithChildrenRemove() { @@ -248,6 +420,51 @@ public class UpdateDataSetTest extends AbstractSampleTest assertRemovingParent(originalChild, originalParent, update); } + @Test + public void testUpdateWithComponentUnauthorized() + { + final String sessionToken = v3api.login(TEST_SPACE_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20120619092259000-22"); + final DataSetPermId containedId = new DataSetPermId("20081105092159111-1"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.getContainedIds().add(containedId); + + assertUnauthorizedObjectAccessException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + } + }, containedId); + } + + @Test + public void testUpdateWithComponentCircularDependency() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20081105092259000-8"); + final DataSetPermId componentId = new DataSetPermId("20081105092259000-9"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.getContainedIds().add(componentId); + update.getContainerIds().add(componentId); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + } + }, "Circular dependency found: 20081105092259000-8"); + } + @SuppressWarnings("unchecked") @Test public void testUpdateWithComponentAddRemove() @@ -310,6 +527,28 @@ public class UpdateDataSetTest extends AbstractSampleTest AssertionUtil.assertCollectionContains(dataSetCodes(map.get(cont3a).getContained()), dataSetId.getPermId()); } + @Test + public void testUpdateWithContainerUnauthorized() + { + final String sessionToken = v3api.login(TEST_SPACE_USER, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("20120619092259000-22"); + final DataSetPermId containerId = new DataSetPermId("20081105092159111-1"); + + final DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + update.getContainerIds().add(containerId); + + assertUnauthorizedObjectAccessException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + } + }, containerId); + } + @SuppressWarnings("unchecked") @Test public void testUpdateWithTagsWithSetAddRemove()