From e0e4bbec8b331726f3082100125915b59b3a1cd4 Mon Sep 17 00:00:00 2001
From: buczekp <buczekp>
Date: Fri, 27 May 2011 08:40:21 +0000
Subject: [PATCH] [LMS-2282] fixed authorization of container data sets
SVN: 21487
---
.../authorization/PredicateExecutor.java | 2 +-
.../generic/server/business/bo/DataBO.java | 74 ++++++++++++++-----
.../server/business/bo/DataSetTable.java | 4 +
.../openbis/generic/shared/dto/DataPE.java | 7 ++
.../result_filter/DataSetGroupLoader.java | 2 +-
5 files changed, 69 insertions(+), 20 deletions(-)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/PredicateExecutor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/PredicateExecutor.java
index 248568bd605..3c215dc0da1 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/PredicateExecutor.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/PredicateExecutor.java
@@ -379,7 +379,7 @@ public final class PredicateExecutor
{
case DATASET:
DataPE dataset = daoFactory.getDataDAO().getByTechId(techId);
- return dataset.getExperiment().getProject().getSpace();
+ return dataset.getSpace();
case EXPERIMENT:
ExperimentPE experiment = daoFactory.getExperimentDAO().getByTechId(techId);
return experiment.getProject().getSpace();
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 698270be290..fa0fcb0b221 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
@@ -56,6 +56,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
import ch.systemsx.cisd.openbis.generic.shared.dto.Session;
+import ch.systemsx.cisd.openbis.generic.shared.dto.SpacePE;
import ch.systemsx.cisd.openbis.generic.shared.dto.StorageFormat;
import ch.systemsx.cisd.openbis.generic.shared.dto.VocabularyPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.VocabularyTermPE;
@@ -210,13 +211,28 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO
public void setContainedDataSets(ExperimentPE experiment, NewContainerDataSet newData)
{
- final List<String> containedDataSetCodes = newData.getContainedDataSetCodes();
- if (containedDataSetCodes != null)
+ SpacePE containerSpace = data.getSpace();
+ // sanity check
+ SpacePE newComponentsSpace = experiment.getProject().getSpace();
+ if (containerSpace.equals(newComponentsSpace) == false)
{
- for (String containedCode : containedDataSetCodes)
+ throw UserFailureException.fromTemplate(
+ "Contained data sets need to be in the same space ('%s') as the container.",
+ containerSpace);
+ } else
+ {
+ if (experiment.equals(data.getExperiment()))
{
- final DataPE contained = getOrCreateData(containedCode, experiment);
- data.addComponent(contained);
+ final List<String> containedDataSetCodes = newData.getContainedDataSetCodes();
+ if (containedDataSetCodes != null)
+ {
+ for (String containedCode : containedDataSetCodes)
+ {
+ final DataPE contained = getOrCreateData(containedCode, experiment);
+ data.addComponent(contained);
+ checkSameSpace(data, contained); // needed for already existing data sets
+ }
+ }
}
}
}
@@ -352,19 +368,19 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO
assert dataSetCode != null : "Unspecified parent data set code.";
final IDataDAO dataDAO = getDataDAO();
- DataPE parent = dataDAO.tryToFindDataSetByCode(dataSetCode);
- if (parent == null)
+ DataPE result = dataDAO.tryToFindDataSetByCode(dataSetCode);
+ if (result == null)
{
- parent = new DataPE();
- parent.setDataStore(dataStore);
- parent.setCode(dataSetCode);
+ result = new DataPE();
+ result.setDataStore(dataStore);
+ result.setCode(dataSetCode);
String code = DataSetTypeCode.UNKNOWN.getCode();
- parent.setDataSetType(getDataSetTypeDAO().tryToFindDataSetTypeByCode(code));
- parent.setExperiment(experiment);
- parent.setPlaceholder(true);
- dataDAO.createDataSet(parent);
+ result.setDataSetType(getDataSetTypeDAO().tryToFindDataSetTypeByCode(code));
+ result.setExperiment(experiment);
+ result.setPlaceholder(true);
+ dataDAO.createDataSet(result);
}
- return parent;
+ return result;
}
public void save() throws UserFailureException
@@ -470,8 +486,10 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO
// remove connection with sample
data.setSample(null);
}
+ checkSameSpace(data, data.getContainer()); // space could be changed by change of experiment
updateParents(updates.getModifiedParentDatasetCodesOrNull());
updateComponents(updates.getModifiedContainedDatasetCodesOrNull());
+ checkSameSpace(data, data.getContainedDataSets()); // even if components were not changed
updateFileFormatType(updates.getFileFormatTypeCode());
updateProperties(data, updates.getProperties());
entityPropertiesConverter.checkMandatoryProperties(data.getProperties(),
@@ -479,6 +497,22 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO
validateAndSave();
}
+ private void checkSameSpace(DataPE container, DataPE component)
+ {
+ // see LMS-2282
+ throw UserFailureException.fromTemplate("Data set's '%s' space ('%s') needs to be the same"
+ + " as its container's '%s' space ('%s').", component.getCode(), component
+ .getSpace().getCode(), container.getCode(), container.getSpace().getCode());
+ }
+
+ private void checkSameSpace(DataPE container, List<DataPE> components)
+ {
+ for (DataPE component : components)
+ {
+ checkSameSpace(container, component);
+ }
+ }
+
private void validateAndSave()
{
getDataDAO().validateAndSaveUpdatedEntity(data);
@@ -714,10 +748,14 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO
data.setSample(newSample);
}
- private void updateExperiment(ExperimentIdentifier experimentIdentifierOrNull)
+ private void updateExperiment(ExperimentIdentifier experimentIdentifier)
{
- assert experimentIdentifierOrNull != null;
- ExperimentPE experiment = getExperimentByIdentifier(experimentIdentifierOrNull);
+ assert experimentIdentifier != null;
+ if (data.getExperiment().getIdentifier().equals(experimentIdentifier.toString()))
+ {
+ return; // nothing to change
+ }
+ ExperimentPE experiment = getExperimentByIdentifier(experimentIdentifier);
data.setExperiment(experiment);
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataSetTable.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataSetTable.java
index c7c8cfcbd69..f7c0ec246ba 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataSetTable.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/DataSetTable.java
@@ -890,6 +890,10 @@ public final class DataSetTable extends AbstractDataSetBusinessObject implements
public void update(List<NewDataSet> updates)
{
+ // NOTE: Only data set properties are currently updatable in batch. If we add possiblity to
+ // batch update assignment to sample/experiment same business rule checks will need to be
+ // performed here as those that are performed when a data set is registered/updated
+ // (see DataBO).
assert updates != null : "Unspecified updates.";
setBatchUpdateMode(true);
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/DataPE.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/DataPE.java
index 2f7f9688df2..c6435f47abd 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/DataPE.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/DataPE.java
@@ -661,4 +661,11 @@ public class DataPE extends AbstractIdAndCodeHolder<DataPE> implements
{
return true;
}
+
+ // convenience method useful when checking authorization rules
+ @Transient
+ public SpacePE getSpace()
+ {
+ return getExperiment().getProject().getSpace();
+ }
}
\ No newline at end of file
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/query/shared/authorization/result_filter/DataSetGroupLoader.java b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/query/shared/authorization/result_filter/DataSetGroupLoader.java
index 776700cb48e..f5c9f253d64 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/query/shared/authorization/result_filter/DataSetGroupLoader.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/query/shared/authorization/result_filter/DataSetGroupLoader.java
@@ -46,7 +46,7 @@ class DataSetGroupLoader implements IGroupLoader
List<DataPE> data = dao.listByCode(keys);
for (DataPE d : data)
{
- map.put(d.getCode(), d.getExperiment().getProject().getSpace());
+ map.put(d.getCode(), d.getSpace());
}
return map;
}
--
GitLab