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 0a59d6c0603c5095e5a3140b2c13d980b520a15b..4a96455c36a1227916d4260bc43c758daa0f2569 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 @@ -32,6 +32,7 @@ import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory; import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDataDAO; import ch.systemsx.cisd.openbis.generic.server.dataaccess.IVocabularyDAO; import ch.systemsx.cisd.openbis.generic.shared.basic.TechId; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Code; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetArchivingStatus; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetType; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.EntityProperty; @@ -572,19 +573,33 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO // quick check for direct cycle final Set<String> brandNewCodes = new HashSet<String>(newCodes); brandNewCodes.removeAll(currentCodes); - if (brandNewCodes.contains(data.getCode())) - { - throw new UserFailureException("Data set '" + data.getCode() - + "' can not be its own component."); - } - // TODO 2011-05-16, Piotr Buczek: validation of container relationship graph - // validateContainerRelationshipGraph(componentsToAdd); + validateContainerRelationshipGraph(brandNewCodes); final List<DataPE> newComponents = findDataSetsByCodes(newCodes); addComponents(newComponents); } } + private void validateContainerRelationshipGraph(Collection<String> componentCodes) + { + if (componentCodes.contains(data.getCode())) + { + throw new UserFailureException("Data set '" + data.getCode() + + "' cannot contain itself as a component neither directly nor via subordinate components."); + } + + final List<DataPE> components = findDataSetsByCodes(componentCodes); + for (DataPE componentDataSet : components) + { + if (componentDataSet.isContainer()) + { + List<DataPE> containedDataSets = componentDataSet.getContainedDataSets(); + validateContainerRelationshipGraph(Code.extractCodes(containedDataSets)); + } + } + + } + /** * Throws {@link UserFailureException} if adding specified parents to this data set will create * a cycle in data set relationships. @@ -685,7 +700,7 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO } } - private List<DataPE> findDataSetsByCodes(Set<String> codes) + private List<DataPE> findDataSetsByCodes(Collection<String> codes) { final IDataDAO dao = getDataDAO(); final List<DataPE> dataSets = new ArrayList<DataPE>(); 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 aa85526a9aaf6f97992c7e0478ca6308b0367260..ec508efd4ec3f576f71f682005802ab9a4d442c4 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 @@ -690,12 +690,53 @@ public class DataBOTest extends AbstractBOTest fail("UserFailureException expected"); } catch (UserFailureException e) { - assertEquals("Data set 'DS1' can not be its own component.", e.getMessage()); + assertEquals("Data set 'DS1' cannot contain itself as a " + + "component neither directly nor via subordinate components.", e.getMessage()); } context.assertIsSatisfied(); } + @Test + public void testUpdateWithContainerRecursivelyContainingItself() + { + final ExperimentPE experiment = + createExperiment(EXPERIMENT_IDENTIFIER.getExperimentCode(), "spaceCode"); + + final DataPE container1 = createDataSet("container-1", null, experiment); + container1.getDataSetType().setContainerType(true); + final DataPE container2 = createDataSet("container-2", null, experiment); + container2.getDataSetType().setContainerType(true); + container2.addComponent(container1); + + DataSetUpdatesDTO dataSetUpdatesDTO = + createDataSetUpdates(container1, null, EXPERIMENT_IDENTIFIER); + String[] componentCodes = + { container2.getCode() }; + dataSetUpdatesDTO.setModifiedContainedDatasetCodesOrNull(componentCodes); + + prepareForUpdate(container1, experiment); + context.checking(new Expectations() + { + { + one(dataDAO).tryToFindDataSetByCode(container2.getCode()); + will(returnValue(container2)); + } + }); + + IDataBO dataBO = createDataBO(); + try + { + dataBO.update(dataSetUpdatesDTO); + fail("UserFailureException expected"); + } catch (UserFailureException e) + { + assertEquals("Data set 'container-1' cannot contain itself as a component" + + " neither directly nor via subordinate components.", e.getMessage()); + } + context.assertIsSatisfied(); + } + private void prepareForUpdate(final ExternalDataPE dataSet, final SamplePE sample) { context.checking(new Expectations()