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 18a714740b638c0c289d9e6a07bbf4bdd85a1c91..4d0b9c5edd9e7445ce1fb4dabe0b3c32c15bff77 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,7 +30,6 @@ 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; @@ -83,13 +82,6 @@ public class UpdateDataSetExperimentExecutor extends @Override protected void update(IOperationContext context, DataPE entity, ExperimentPE related) { - if (related == null) - { - throw new UserFailureException("Data set '" + entity.getCode() + "' cannot have experiment set to null"); - } else - { - relationshipService.assignDataSetToExperiment(context.getSession(), entity, related); - } + this.BOfactory.createDataBO(context.getSession()).assignDataSetToSampleAndExperiment(entity, null, related); } - } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetSampleExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetSampleExecutor.java index 8888e6f66109319c96c4b69e6a98f34c55d94a82..e602d4aed6aebb1cdd6baeb69c30bbc3b8b660ed 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetSampleExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/dataset/UpdateDataSetSampleExecutor.java @@ -30,7 +30,6 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.dataset.DataSetUpdat 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.exceptions.UnauthorizedObjectAccessException; -import ch.systemsx.cisd.common.exceptions.UserFailureException; import ch.systemsx.cisd.openbis.generic.server.authorization.validator.SampleByIdentiferValidator; import ch.systemsx.cisd.openbis.generic.shared.dto.DataPE; import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE; @@ -39,7 +38,8 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE; * @author pkupczyk */ @Component -public class UpdateDataSetSampleExecutor extends AbstractUpdateEntityFieldUpdateValueRelationExecutor<DataSetUpdate, DataPE, ISampleId, SamplePE> implements +public class UpdateDataSetSampleExecutor extends AbstractUpdateEntityFieldUpdateValueRelationExecutor<DataSetUpdate, DataPE, ISampleId, SamplePE> + implements IUpdateDataSetSampleExecutor { @@ -73,18 +73,6 @@ public class UpdateDataSetSampleExecutor extends AbstractUpdateEntityFieldUpdate @Override protected void check(IOperationContext context, DataPE entity, ISampleId relatedId, SamplePE related) { - if (related.getSpace() == null) - { - throw new UserFailureException("Data set '" + entity.getCode() + "' cannot be connected to a shared sample '" + related.getIdentifier() - + "'"); - } - - if (related.getExperiment() == null) - { - throw new UserFailureException("Data set '" + entity.getCode() + "' cannot be connected to a sample '" + related.getIdentifier() - + "' that does not have an experiment"); - } - if (false == new SampleByIdentiferValidator().doValidation(context.getSession().tryGetPerson(), related)) { throw new UnauthorizedObjectAccessException(relatedId); @@ -94,12 +82,12 @@ public class UpdateDataSetSampleExecutor extends AbstractUpdateEntityFieldUpdate @Override protected void update(IOperationContext context, DataPE entity, SamplePE related) { - if (related == null) + if (related != null) { - entity.setSample(null); + this.BOfactory.createDataBO(context.getSession()).assignDataSetToSampleAndExperiment(entity, related, related.getExperiment()); } else { - relationshipService.assignDataSetToSample(context.getSession(), entity, related); + this.BOfactory.createDataBO(context.getSession()).assignDataSetToSampleAndExperiment(entity, null, entity.getExperiment()); } } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/entity/AbstractUpdateEntityFieldUpdateValueRelationExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/entity/AbstractUpdateEntityFieldUpdateValueRelationExecutor.java index 960f3313b319a15f49e20cc9650ecceebce7221d..4811aa8968a6a2f5a991095bb44e3757ee71381d 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/entity/AbstractUpdateEntityFieldUpdateValueRelationExecutor.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/executor/entity/AbstractUpdateEntityFieldUpdateValueRelationExecutor.java @@ -27,6 +27,7 @@ import ch.ethz.sis.openbis.generic.shared.api.v3.dto.entity.FieldUpdateValue; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.IObjectId; import ch.ethz.sis.openbis.generic.shared.api.v3.exceptions.ObjectNotFoundException; import ch.systemsx.cisd.openbis.generic.server.business.IRelationshipService; +import ch.systemsx.cisd.openbis.generic.server.business.bo.ICommonBusinessObjectFactory; /** * @author pkupczyk @@ -39,6 +40,9 @@ public abstract class AbstractUpdateEntityFieldUpdateValueRelationExecutor<ENTIT @Autowired protected IRelationshipService relationshipService; + @Autowired + protected ICommonBusinessObjectFactory BOfactory; + @Override public void update(IOperationContext context, Map<ENTITY_UPDATE, ENTITY_PE> entitiesMap) { diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractBusinessObject.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractBusinessObject.java index 5c966c6a8b8ba53c8cbefc52ead9af7bbf074537..884e1a27214c88d693b740b58333274e7bae79c7 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractBusinessObject.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractBusinessObject.java @@ -37,6 +37,7 @@ import ch.systemsx.cisd.common.exceptions.UserFailureException; import ch.systemsx.cisd.openbis.generic.server.business.IDataStoreServiceFactory; import ch.systemsx.cisd.openbis.generic.server.business.IRelationshipService; import ch.systemsx.cisd.openbis.generic.server.business.bo.util.DataSetTypeWithoutExperimentChecker; +import ch.systemsx.cisd.openbis.generic.server.business.bo.util.SampleUtils; import ch.systemsx.cisd.openbis.generic.server.dataaccess.EntityPropertiesConverter; import ch.systemsx.cisd.openbis.generic.server.dataaccess.IAttachmentDAO; import ch.systemsx.cisd.openbis.generic.server.dataaccess.IAuthorizationGroupDAO; @@ -797,6 +798,19 @@ abstract class AbstractBusinessObject implements IDAOFactory protected void assignDataSetToSampleAndExperiment(DataPE data, SamplePE newSample, ExperimentPE experiment) { + if (newSample != null) + { + SamplePE previousSampleOrNull = data.tryGetSample(); + if (newSample.equals(previousSampleOrNull)) + { + return; // nothing to change + } + if (newSample.getSpace() == null) + { + throw SampleUtils.createWrongSampleException(data, newSample, "the new sample is shared"); + } + } + NewDataSetToSampleExperimentAssignmentManager assignmentManager = new NewDataSetToSampleExperimentAssignmentManager(dataSetTypeChecker); assignmentManager.assignDataSetAndRelatedComponents(data, newSample, experiment); assignmentManager.performAssignment(relationshipService, session); diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractDataSetBusinessObject.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractDataSetBusinessObject.java index bf0d5a703993d17c317f5986068d8e38d00577a8..e3ae23eef5ab91bf57518af3b39fd06d28802f79 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractDataSetBusinessObject.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractDataSetBusinessObject.java @@ -32,7 +32,6 @@ import ch.systemsx.cisd.common.exceptions.UserFailureException; import ch.systemsx.cisd.openbis.generic.server.business.IRelationshipService; import ch.systemsx.cisd.openbis.generic.server.business.IServiceConversationClientManagerLocal; import ch.systemsx.cisd.openbis.generic.server.business.bo.util.DataSetTypeWithoutExperimentChecker; -import ch.systemsx.cisd.openbis.generic.server.business.bo.util.SampleUtils; 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.IEntityPropertiesConverter; @@ -104,16 +103,6 @@ public abstract class AbstractDataSetBusinessObject extends AbstractSampleIdenti { assert sampleIdentifierOrNull != null; SamplePE newSample = getSampleByIdentifier(sampleIdentifierOrNull); - SamplePE previousSampleOrNull = data.tryGetSample(); - - if (newSample.equals(previousSampleOrNull)) - { - return; // nothing to change - } - if (newSample.getSpace() == null) - { - throw SampleUtils.createWrongSampleException(data, newSample, "the new sample is shared"); - } assignDataSetToSampleAndExperiment(data, newSample, newSample.getExperiment()); } 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 4a6ca6445ee51bde174fc21be16d7f8746719950..5a88d147afc0063fafee8aec30642872ffc0a04f 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 @@ -230,8 +230,8 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO } ExperimentPE experiment = sample.getExperiment(); + RelationshipUtils.setSampleForDataSet(data, sample, session); - RelationshipUtils.setExperimentForDataSet(data, experiment, session); setParentDataSets(experiment, sample, newData); } @@ -603,6 +603,12 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO return propertiesToUpdate; } + @Override + public void assignDataSetToSampleAndExperiment(DataPE data, SamplePE sample, ExperimentPE experiment) + { + super.assignDataSetToSampleAndExperiment(data, sample, experiment); + } + @Override public void update(DataSetUpdatesDTO updates) { @@ -631,8 +637,7 @@ public class DataBO extends AbstractDataSetBusinessObject implements IDataBO entityPropertiesConverter.checkMandatoryProperties(data.getProperties(), data.getDataSetType()); - - data.setModificationDate(new Date(data.getModificationDate().getTime() + 1)); + data.setModificationDate(new Date(data.getModificationDate().getTime() + 1)); validateAndSave(); data.setModificationDate(new Date(data.getModificationDate().getTime() - 1)); diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/IDataBO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/IDataBO.java index 1049ac3ecfd5b516749d04dd8b0af6efdddb7eef..ea45cee532d2e3ce32cc978dedd22d269e32c599 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/IDataBO.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/IDataBO.java @@ -31,6 +31,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.NewContainerDataSet; import ch.systemsx.cisd.openbis.generic.shared.dto.NewExternalData; import ch.systemsx.cisd.openbis.generic.shared.dto.NewProperty; import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE; +import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifier; /** * @author Franz-Josef Elmer @@ -146,4 +147,8 @@ public interface IDataBO extends IEntityBusinessObject */ public boolean isStorageConfirmed(); + /** + * Set sample and experiment of a specified data set + */ + public void assignDataSetToSampleAndExperiment(DataPE data, SamplePE sample, ExperimentPE experiment); } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/util/RelationshipUtils.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/util/RelationshipUtils.java index 820f996a26df69ab9d5a55c5a53e9cab999f9941..5f713cf567b63aaf565f5316669df739fd5fbb5c 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/util/RelationshipUtils.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/util/RelationshipUtils.java @@ -156,6 +156,21 @@ public class RelationshipUtils { updateModificationDateAndModifier(dataSet.tryGetSample(), session); dataSet.setSample(sample); + if (sample != null) + { + if (dataSet.getExperiment() != sample.getExperiment()) + { + if (dataSet.getExperiment() != null) + { + updateModificationDateAndModifier(dataSet.getExperiment(), session); + } + if (sample.getExperiment() != null) + { + updateModificationDateAndModifier(sample.getExperiment(), session); + } + } + dataSet.setExperiment(sample.getExperiment()); + } updateModificationDateAndModifier(sample, session); updateModificationDateAndModifier(dataSet, session); } @@ -174,6 +189,7 @@ public class RelationshipUtils { updateModificationDateAndModifier(dataSet.getExperiment(), session); dataSet.setExperiment(experiment); + dataSet.setSample(null); updateModificationDateAndModifier(experiment, session); updateModificationDateAndModifier(dataSet, session); } diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/AssignDataSetToExperimentAndSampleTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/AssignDataSetToExperimentAndSampleTest.java index 10325ed57483b4a351e9d1bed6ba1026841af692..0c4953799bc1a7c28ad9a14f61621b52baf813ab 100644 --- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/AssignDataSetToExperimentAndSampleTest.java +++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/api/v3/AssignDataSetToExperimentAndSampleTest.java @@ -33,40 +33,34 @@ import ch.systemsx.cisd.openbis.systemtest.AbstractDataSetAssignmentTestCase; * * @author Franz-Josef Elmer */ -@Test(groups = { "system-cleandb", "broken" }) +@Test(groups = { "system-cleandb" }) public class AssignDataSetToExperimentAndSampleTest extends AbstractDataSetAssignmentTestCase { - @Autowired protected IApplicationServerApi v3api; @Override protected void reassignToExperiment(String dataSetCode, String experimentIdentifierOrNull, String userSessionToken) { - reassignDataSet(dataSetCode, experimentIdentifierOrNull, null, userSessionToken); + DataSetUpdate dataSetUpdate = new DataSetUpdate(); + dataSetUpdate.setDataSetId(new DataSetPermId(dataSetCode)); + dataSetUpdate.setExperimentId(new ExperimentIdentifier(experimentIdentifierOrNull)); + v3api.updateDataSets(userSessionToken, Arrays.asList(dataSetUpdate)); } @Override protected void reassignToSample(String dataSetCode, String samplePermIdOrNull, String userSessionToken) - { - reassignDataSet(dataSetCode, null, samplePermIdOrNull, userSessionToken); - } - - private void reassignDataSet(String dataSetCode, String experimentIdentifierOrNull, String samplePermIdOrNull, - String userSessionToken) { DataSetUpdate dataSetUpdate = new DataSetUpdate(); dataSetUpdate.setDataSetId(new DataSetPermId(dataSetCode)); - if (experimentIdentifierOrNull != null) + if (samplePermIdOrNull == null) { - dataSetUpdate.setExperimentId(new ExperimentIdentifier(experimentIdentifierOrNull)); + dataSetUpdate.setSampleId(null); } - if (samplePermIdOrNull != null) + else { dataSetUpdate.setSampleId(new SamplePermId(samplePermIdOrNull)); } v3api.updateDataSets(userSessionToken, Arrays.asList(dataSetUpdate)); - } - } 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 822b484663532290e39f6ba547f90d61f3b8d743..ec63047ed70cc469a795c066394478c196216242 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 @@ -45,6 +45,7 @@ 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; import ch.systemsx.cisd.common.test.AssertionUtil; /** @@ -127,13 +128,36 @@ public class UpdateDataSetTest extends AbstractSampleTest DataSetFetchOptions fe = new DataSetFetchOptions(); fe.withSample(); + fe.withExperiment(); Map<IDataSetId, DataSet> map = v3api.mapDataSets(sessionToken, Arrays.asList(dataSetId), fe); DataSet result = map.get(dataSetId); assertEquals(result.getSample().getPermId(), sampleId); + assertEquals(result.getExperiment().getCode(), "EXP-SPACE-TEST"); } - // @Test broken + @Test(expectedExceptions = { UserFailureException.class }, expectedExceptionsMessageRegExp = "Access denied.*") + public void testUpdateWithSampleNotAllowed() + { + String sessionToken = v3api.login(TEST_POWER_USER_CISD, PASSWORD); + + DataSetPermId dataSetId = new DataSetPermId("COMPONENT_1A"); + + DataSetUpdate update = new DataSetUpdate(); + update.setDataSetId(dataSetId); + SamplePermId sampleId = new SamplePermId("200902091250077-1060"); + update.setSampleId(sampleId); + v3api.updateDataSets(sessionToken, Collections.singletonList(update)); + + DataSetFetchOptions fe = new DataSetFetchOptions(); + fe.withSample(); + Map<IDataSetId, DataSet> map = v3api.mapDataSets(sessionToken, Arrays.asList(dataSetId), fe); + + DataSet result = map.get(dataSetId); + assertEquals(result.getSample().getPermId(), sampleId); + } + + @Test public void testUpdateWithSampleWithoutAnExperiment() { String sessionToken = v3api.login(TEST_USER, PASSWORD); @@ -265,7 +289,7 @@ public class UpdateDataSetTest extends AbstractSampleTest { v3api.updateDataSets(sessionToken, Collections.singletonList(update)); } - }, "Data set '20081105092259000-18' cannot have experiment set to null"); + }, "Neither experiment nor sample is specified for data set 20081105092259000-18"); } @Test diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/AbstractDataSetAssignmentTestCase.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/AbstractDataSetAssignmentTestCase.java index 203c5d0646252e031d95a52d1e1f402b58b17e12..88c4ccdaddc14c18b7d4880dbed39c90459a8981 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/AbstractDataSetAssignmentTestCase.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/AbstractDataSetAssignmentTestCase.java @@ -25,8 +25,8 @@ import org.testng.annotations.BeforeClass; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; -import ch.systemsx.cisd.common.exceptions.AuthorizationFailureException; import ch.systemsx.cisd.common.exceptions.UserFailureException; +import ch.systemsx.cisd.common.test.AssertionUtil; import ch.systemsx.cisd.openbis.generic.server.business.bo.entitygraph.DataSetNode; import ch.systemsx.cisd.openbis.generic.server.business.bo.entitygraph.EntityGraphGenerator; import ch.systemsx.cisd.openbis.generic.server.business.bo.entitygraph.ExperimentNode; @@ -46,11 +46,9 @@ import ch.systemsx.cisd.openbis.systemtest.base.auth.RolePermutator; import ch.systemsx.cisd.openbis.systemtest.base.auth.SpaceDomain; /** - * Abstract super class for all tests assigning a data set to a sample or an experiment. - * Subclasses for the different versions of the API have only to implement - * {@link #reassignToExperiment(String, String, String)} and - * {@link #reassignToSample(String, String, String)}. - * + * Abstract super class for all tests assigning a data set to a sample or an experiment. Subclasses for the different versions of the API have only to + * implement {@link #reassignToExperiment(String, String, String)} and {@link #reassignToSample(String, String, String)}. + * * @author anttil * @author Franz-Josef Elmer */ @@ -68,7 +66,7 @@ public abstract class AbstractDataSetAssignmentTestCase extends BaseTest Space sourceSpace; Space destinationSpace; - + Space unrelatedAdmin; Space unrelatedObserver; @@ -813,7 +811,7 @@ public abstract class AbstractDataSetAssignmentTestCase extends BaseTest "not connected to any experiment and the data set type (" + dataset.getDataSetType().getCode() + ") doesn't match one of the following regular expressions: NO-EXP-.* , NE.* ."; - assertEquals("The dataset '" + dataset.getCode() + AssertionUtil.assertStarts("The dataset '" + dataset.getCode() + "' cannot be connected to the sample '" + sample.getIdentifier() + "' because the new sample is " + postfix, ex.getMessage()); }