diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/VerifySampleContainerExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/VerifySampleContainerExecutor.java index 42981cab766de9779a30fef6b3bb542bc343f577..8d3e6bb0c02ba70a25814d76c58b018b309a8a45 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/VerifySampleContainerExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/VerifySampleContainerExecutor.java @@ -50,6 +50,7 @@ public class VerifySampleContainerExecutor implements IVerifySampleContainerExec } SampleGenericBusinessRules.assertValidContainer(sample); + SampleGenericBusinessRules.assertValidComponents(sample); } } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/VerifySampleParentsExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/VerifySampleParentsExecutor.java index 29dc12b4b54898d227c91ee16da5151e06583629..d2be953b4eb0c0b8b3751549565038f47b0764eb 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/VerifySampleParentsExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/sample/VerifySampleParentsExecutor.java @@ -65,7 +65,7 @@ public class VerifySampleParentsExecutor implements IVerifySampleParentsExecutor { Map<Long, Collection<Long>> graph = getGraph(context, samples); - checkCycles(samples, graph); + checkCycles(graph); for (SamplePE sample : samples) { @@ -113,20 +113,15 @@ public class VerifySampleParentsExecutor implements IVerifySampleParentsExecutor return parentIdsMap; } - private void checkCycles(Collection<SamplePE> samples, Map<Long, Collection<Long>> graph) + private void checkCycles(Map<Long, Collection<Long>> graph) { try { GroupingDAG.groupByDepencies(graph); } catch (CycleFoundException e) { - Map<Long, SamplePE> sampleMap = new HashMap<Long, SamplePE>(); - for (SamplePE sample : samples) - { - sampleMap.put(sample.getId(), sample); - } - - throw new UserFailureException("Circular parent dependency found for sample: " + (sampleMap.get(e.getCycleRoot()).getIdentifier()), e); + SamplePE sample = daoFactory.getSampleDAO().getByTechId(new TechId((Long) e.getCycleRoot())); + throw new UserFailureException("Circular parent dependency found for sample: " + (sample.getIdentifier()), e); } } } diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/UpdateSampleTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/UpdateSampleTest.java index c76faf46a4ddbadf56582ff5a112f4b36f1acd4f..a55e35ced01e54cce39507ae7624623893444bcc 100644 --- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/UpdateSampleTest.java +++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/UpdateSampleTest.java @@ -39,6 +39,7 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.entitytype.EntityTypePer 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.ISampleId; +import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.sample.SampleIdentifier; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.sample.SamplePermId; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.space.ISpaceId; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.space.SpacePermId; @@ -262,6 +263,25 @@ public class UpdateSampleTest extends AbstractSampleTest }); } + @Test + public void testUpdateWithSpaceNullForSpaceSampleWithDataSets() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SampleIdentifier("/CISD/CP-TEST-1")); + update.setSpaceId(null); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "Cannot detach the sample '/CP-TEST-1' from the space because there are already datasets attached to the sample"); + } + @Test public void testUpdateWithSpaceNotNullForSharedSampleAsAdminUser() { @@ -406,7 +426,7 @@ public class UpdateSampleTest extends AbstractSampleTest } @Test - public void testUpdateWithExperimentNull() + public void testUpdateWithExperimentNullForSampleWithoutDataSets() { String sessionToken = v3api.login(TEST_USER, PASSWORD); @@ -436,6 +456,25 @@ public class UpdateSampleTest extends AbstractSampleTest Assert.assertNull(sample.getExperiment()); } + @Test + public void testUpdateWithExperimentNullForSampleWithDataSets() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SampleIdentifier("/CISD/CP-TEST-1")); + update.setExperimentId(null); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "Cannot detach the sample '/CISD/CP-TEST-1' from the experiment because there are already datasets attached to the sample"); + } + @Test public void testUpdateWithExperimentUnauthorized() { @@ -599,6 +638,44 @@ public class UpdateSampleTest extends AbstractSampleTest }, containerId); } + @Test + public void testUpdateWithContainerCircularDependency() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SamplePermId("200811050919915-8")); + update.setContainerId(new SamplePermId("200811050919915-9")); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "'/CISD/A01:CL1' cannot be it's own container"); + } + + @Test + public void testUpdateWithContainerViolatingBusinessRules() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SampleIdentifier("/MP")); + update.setContainerId(new SampleIdentifier("/CISD/3V-125")); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "The database instance sample '/3V-125:MP' can not be contained in the space sample '/CISD/3V-125"); + } + @Test public void testUpdateWithContainedSetAddRemove() { @@ -713,7 +790,45 @@ public class UpdateSampleTest extends AbstractSampleTest } @Test - public void testUpdateWithParentsSetAddRemove() + public void testUpdateWithContainedCircularDependency() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SamplePermId("200811050919915-9")); + update.getContainedIds().add(new SamplePermId("200811050919915-8")); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "'/CISD/CL1:A01' cannot be it's own container"); + } + + @Test + public void testUpdateWithContainedViolatingBusinessRules() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SampleIdentifier("/CISD/3V-125")); + update.getContainedIds().add(new SampleIdentifier("/MP")); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "Sample '/CISD/3V-125' can not be a space sample because of a contained database instance sample '/3V-125:MP"); + } + + @Test + public void testUpdateWithParentSetAddRemove() { String sessionToken = v3api.login(TEST_USER, PASSWORD); @@ -825,7 +940,45 @@ public class UpdateSampleTest extends AbstractSampleTest } @Test - public void testUpdateWithChildrenSetAddRemove() + public void testUpdateWithParentCircularDependency() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SamplePermId("200811050945092-976")); + update.getParentIds().add(new SamplePermId("200811050946559-982")); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "Circular parent dependency found for sample: /CISD/3VCP8"); + } + + @Test + public void testUpdateWithParentViolatingBusinessRules() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SampleIdentifier("/MP")); + update.getParentIds().add(new SampleIdentifier("/CISD/3V-125")); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "The database instance sample '/MP' can not be child of the space sample '/CISD/3V-125'"); + } + + @Test + public void testUpdateWithChildSetAddRemove() { String sessionToken = v3api.login(TEST_USER, PASSWORD); @@ -936,6 +1089,44 @@ public class UpdateSampleTest extends AbstractSampleTest }, childId); } + @Test + public void testUpdateWithChildCircularDependency() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SamplePermId("200811050946559-982")); + update.getChildIds().add(new SamplePermId("200811050945092-976")); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "Circular parent dependency found for sample: /CISD/3VCP8"); + } + + @Test + public void testUpdateWithChildViolatingBusinessRules() + { + final String sessionToken = v3api.login(TEST_USER, PASSWORD); + + final SampleUpdate update = new SampleUpdate(); + update.setSampleId(new SampleIdentifier("/CISD/3V-125")); + update.getChildIds().add(new SampleIdentifier("/MP")); + + assertUserFailureException(new IDelegatedAction() + { + @Override + public void execute() + { + v3api.updateSamples(sessionToken, Arrays.asList(update)); + } + }, "Sample '/CISD/3V-125' can not be a space sample because of a child database instance sample '/MP'"); + } + @Test public void testUpdateWithTagsWithSetAddRemove() {