From 78347ace2bd6d0b4539dc87ab52c8cc3d498d6a8 Mon Sep 17 00:00:00 2001
From: felmer <felmer>
Date: Wed, 14 Nov 2012 07:09:05 +0000
Subject: [PATCH] SP-367, BIS-242: Improved handling of simultaneous
 adding/removing of samples in experiment edit form.

SVN: 27604
---
 .../web/client/application/ui/CodesArea.java  |  11 +-
 .../server/business/bo/ExperimentBO.java      |  67 ++--
 .../basic/dto/BasicExperimentUpdates.java     |  12 +
 ...ractGenericExperimentRegisterEditForm.java |   5 +
 .../experiment/GenericExperimentEditForm.java |   1 +
 .../web/server/GenericClientService.java      |   1 +
 .../server/business/bo/ExperimentBOTest.java  | 300 ++++++++++--------
 .../generic/shared/CommonTestUtils.java       |   1 +
 .../ExperimentOptimisticLockingTest.java      |  75 +++--
 .../optimistic_locking/ToolBox.java           |  19 +-
 10 files changed, 302 insertions(+), 190 deletions(-)

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/CodesArea.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/CodesArea.java
index 4e017773638..c207df9d5ae 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/CodesArea.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/client/web/client/application/ui/CodesArea.java
@@ -31,6 +31,8 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Code;
  */
 abstract public class CodesArea<T extends ICodeHolder> extends MultilineItemsField
 {
+    private List<String> originalCodes;
+
     public CodesArea(String emptyTextMsg)
     {
         super("", false);
@@ -39,8 +41,13 @@ abstract public class CodesArea<T extends ICodeHolder> extends MultilineItemsFie
 
     public final void setCodeProviders(Collection<T> codeProviders)
     {
-        List<String> codes = Code.extractCodes(codeProviders);
-        setItems(codes);
+        originalCodes = Code.extractCodes(codeProviders);
+        setItems(originalCodes);
+    }
+
+    public String[] getOriginalCodes()
+    {
+        return originalCodes.toArray(new String[0]);
     }
 
 }
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBO.java
index 710d07404c3..cd657659ec0 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBO.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBO.java
@@ -40,7 +40,6 @@ import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.id.experiment.Experime
 import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.id.experiment.ExperimentTechIdId;
 import ch.systemsx.cisd.openbis.generic.shared.api.v1.dto.id.experiment.IExperimentId;
 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.IEntityProperty;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewAttachment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewExperiment;
@@ -492,54 +491,35 @@ public final class ExperimentBO extends AbstractBusinessObject implements IExper
                 attachSamples(sampleCodes);
             } else
             {
-                setExperimentSamples(sampleCodes);
+                String[] originalSampleCodes = updates.getOriginalSampleCodes();
+                updateSamples(originalSampleCodes, sampleCodes);
             }
         }
     }
 
-    @Private
-    // attaches specified existing samples to the experiment
-    void attachSamples(String[] sampleCodes)
+    private void updateSamples(String[] originalSampleCodes, String[] sampleCodes)
     {
-        List<SamplePE> samplesToAdd = findUnassignedSamplesByCodes(asSet(sampleCodes));
-        addToExperiment(samplesToAdd);
+        Set<String> samplesToAdd = asSet(sampleCodes);
+        samplesToAdd.removeAll(Arrays.asList(originalSampleCodes));
+        addToExperiment(findSamplesByCodes(samplesToAdd, true));
+
+        Set<String> samplesToRemove = asSet(originalSampleCodes);
+        samplesToRemove.removeAll(Arrays.asList(sampleCodes));
+        removeFromExperiment(findSamplesByCodes(samplesToRemove, false));
     }
 
     @Private
-    // changes the list of samples assigned to this experiment to the specified one
-    void setExperimentSamples(String[] sampleCodes)
+    // attaches specified existing samples to the experiment
+    void attachSamples(String[] sampleCodes)
     {
-        List<SamplePE> samples = experiment.getSamples();
-        String[] currentSampleCodes = Code.extractCodesToArray(samples);
-        Set<String> currentSampleCodesSet = asSet(currentSampleCodes);
-        Set<String> codesToAdd = asSet(sampleCodes);
-        codesToAdd.removeAll(currentSampleCodesSet);
-
-        List<SamplePE> samplesToAdd = findUnassignedSamplesByCodes(codesToAdd);
+        List<SamplePE> samplesToAdd = findSamplesByCodes(asSet(sampleCodes), true);
         addToExperiment(samplesToAdd);
-
-        Set<String> codesToRemove = asSet(currentSampleCodes);
-        codesToRemove.removeAll(asSet(sampleCodes));
-        removeFromExperiment(filterSamples(samples, codesToRemove));
     }
 
-    private List<SamplePE> findUnassignedSamplesByCodes(Set<String> codesToAdd)
+    private List<SamplePE> findSamplesByCodes(Set<String> codesToAdd, boolean unassigned)
     {
-        SpacePE group = experiment.getProject().getSpace();
-        return findUnassignedSamples(getSampleDAO(), codesToAdd, group);
-    }
-
-    private static List<SamplePE> filterSamples(List<SamplePE> samples, Set<String> extractedCodes)
-    {
-        List<SamplePE> result = new ArrayList<SamplePE>();
-        for (SamplePE sample : samples)
-        {
-            if (extractedCodes.contains(sample.getCode()))
-            {
-                result.add(sample);
-            }
-        }
-        return result;
+        SpacePE space = experiment.getProject().getSpace();
+        return findSamples(getSampleDAO(), codesToAdd, space, unassigned);
     }
 
     private void removeFromExperiment(List<SamplePE> samples)
@@ -560,22 +540,25 @@ public final class ExperimentBO extends AbstractBusinessObject implements IExper
         }
     }
 
-    // Finds samples in the specified group. Throws exception if some samples do not exist.
+    // Finds samples in the specified space. Throws exception if some samples do not exist.
     // Throws exception if any sample code specified is already assigned to an experiment.
-    private static List<SamplePE> findUnassignedSamples(ISampleDAO sampleDAO,
-            Set<String> sampleCodes, SpacePE group) throws UserFailureException
+    private static List<SamplePE> findSamples(ISampleDAO sampleDAO, Set<String> sampleCodes,
+            SpacePE space, boolean unassigned) throws UserFailureException
     {
         List<SamplePE> samples = new ArrayList<SamplePE>();
         List<String> missingSamples = new ArrayList<String>();
         for (String code : sampleCodes)
         {
-            SamplePE sample = sampleDAO.tryFindByCodeAndSpace(code, group);
+            SamplePE sample = sampleDAO.tryFindByCodeAndSpace(code, space);
             if (sample == null)
             {
                 missingSamples.add(code);
             } else
             {
-                checkSampleUnassigned(code, sample);
+                if (unassigned)
+                {
+                    checkSampleUnassigned(code, sample);
+                }
                 samples.add(sample);
             }
         }
@@ -583,7 +566,7 @@ public final class ExperimentBO extends AbstractBusinessObject implements IExper
         {
             throw UserFailureException.fromTemplate(
                     "Samples with following codes do not exist in the space '%s': '%s'.",
-                    group.getCode(), CollectionUtils.abbreviate(missingSamples, 10));
+                    space.getCode(), CollectionUtils.abbreviate(missingSamples, 10));
         } else
         {
             return samples;
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/BasicExperimentUpdates.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/BasicExperimentUpdates.java
index 610ee016645..5218e5cca0a 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/BasicExperimentUpdates.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/BasicExperimentUpdates.java
@@ -43,6 +43,8 @@ public class BasicExperimentUpdates implements Serializable
     // if it does not have data sets attached yet. Otherwise an exception will be thrown.
     private String[] sampleCodesOrNull;
 
+    private String[] originalSampleCodes;
+
     // if true sampleCodesOrNull changes semantic - sample codes are the same as in newSamples and
     // all of them will be attached to the experiment.
     private boolean registerSamples;
@@ -82,6 +84,16 @@ public class BasicExperimentUpdates implements Serializable
         this.sampleCodesOrNull = sampleCodes;
     }
 
+    public String[] getOriginalSampleCodes()
+    {
+        return originalSampleCodes;
+    }
+
+    public void setOriginalSampleCodes(String[] originalSampleCodes)
+    {
+        this.originalSampleCodes = originalSampleCodes;
+    }
+
     public void setNewSamples(List<NewSamplesWithTypes> list)
     {
         this.newSamples = list;
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/client/application/experiment/AbstractGenericExperimentRegisterEditForm.java b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/client/application/experiment/AbstractGenericExperimentRegisterEditForm.java
index 7ac62518950..e77283a4b83 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/client/application/experiment/AbstractGenericExperimentRegisterEditForm.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/client/application/experiment/AbstractGenericExperimentRegisterEditForm.java
@@ -275,6 +275,11 @@ abstract public class AbstractGenericExperimentRegisterEditForm extends
             return null;
     }
 
+    protected String[] getOriginalSamples()
+    {
+        return samplesArea != null ? samplesArea.getOriginalCodes() : new String[0];
+    }
+
     protected SampleType getSampleType()
     {
         if (existingSamplesRadio.getValue() == false)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/client/application/experiment/GenericExperimentEditForm.java b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/client/application/experiment/GenericExperimentEditForm.java
index b9ff6791d8d..fc39ed20b95 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/client/application/experiment/GenericExperimentEditForm.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/client/application/experiment/GenericExperimentEditForm.java
@@ -96,6 +96,7 @@ public final class GenericExperimentEditForm extends AbstractGenericExperimentRe
         updates.setProjectIdentifier(extractProjectIdentifier());
         updates.setAttachmentSessionKey(attachmentsSessionKey);
         updates.setAttachments(attachmentsManager.extractAttachments());
+        updates.setOriginalSampleCodes(getOriginalSamples());
         updates.setSampleCodes(getSamples());
         updates.setSampleType(getSampleType());
         updates.setGenerateCodes(autoGenerateCodes.getValue().booleanValue());
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/server/GenericClientService.java b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/server/GenericClientService.java
index b2d429103e6..336f6578ee6 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/server/GenericClientService.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/client/web/server/GenericClientService.java
@@ -607,6 +607,7 @@ public class GenericClientService extends AbstractClientService implements IGene
         updatesDTO.setProjectIdentifier(project);
         updatesDTO.setAttachments(attachments);
         updatesDTO.setProperties(updates.getProperties());
+        updatesDTO.setOriginalSampleCodes(updates.getOriginalSampleCodes());
         updatesDTO.setSampleCodes(updates.getSampleCodes());
         updatesDTO.setVersion(updates.getVersion());
         updatesDTO.setRegisterSamples(updates.isRegisterSamples());
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBOTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBOTest.java
index 8ddb532349a..826687a8c67 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBOTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBOTest.java
@@ -35,7 +35,6 @@ import org.testng.annotations.Test;
 
 import ch.rinn.restrictions.Friend;
 import ch.systemsx.cisd.common.exceptions.UserFailureException;
-import ch.systemsx.cisd.common.test.AssertionUtil;
 import ch.systemsx.cisd.openbis.generic.server.business.ManagerTestTool;
 import ch.systemsx.cisd.openbis.generic.server.util.TimeIntervalChecker;
 import ch.systemsx.cisd.openbis.generic.shared.CommonTestUtils;
@@ -45,6 +44,7 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewAttachment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewExperiment;
 import ch.systemsx.cisd.openbis.generic.shared.dto.AttachmentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.EntityPropertyPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.EntityTypePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.EntityTypePropertyTypePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
@@ -52,7 +52,6 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPropertyPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentTypePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentTypePropertyTypePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentUpdatesDTO;
-import ch.systemsx.cisd.openbis.generic.shared.dto.IAuthSession;
 import ch.systemsx.cisd.openbis.generic.shared.dto.IEntityPropertiesHolder;
 import ch.systemsx.cisd.openbis.generic.shared.dto.IModifierAndModificationDateBean;
 import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE;
@@ -60,6 +59,7 @@ 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.SpacePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifier;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ProjectIdentifier;
 import ch.systemsx.cisd.openbis.generic.shared.dto.properties.EntityKind;
 
@@ -700,18 +700,10 @@ public final class ExperimentBOTest extends AbstractBOTest
         updates.setProperties(Collections.<IEntityProperty> emptyList());
         updates.setAttachments(Collections.<NewAttachment> emptyList());
         prepareAnyDaoCreation();
+        prepareLoadingExperiment(exp);
         context.checking(new Expectations()
             {
                 {
-                    one(experimentDAO).tryGetByTechId(new TechId(exp.getId()),
-                            ExperimentBO.PROPERTY_TYPES);
-                    will(returnValue(exp));
-
-                    one(entityTypeDAO).listEntityTypes();
-                    will(returnValue(Arrays.asList(exp.getExperimentType())));
-
-                    allowing(entityPropertyTypeDAO)
-                            .listEntityPropertyTypes(exp.getExperimentType());
                     one(relationshipService).assignExperimentToProject(
                             ManagerTestTool.EXAMPLE_SESSION, exp, newProject);
                 }
@@ -727,137 +719,210 @@ public final class ExperimentBOTest extends AbstractBOTest
     }
 
     @Test
-    public final void testEditSamples()
+    public void testUpdateByAddingASample()
     {
-        // we test if this sample will stay assigned to the experiment if it was assigned before
-        SamplePE untouchedSample = createSampleWithCode("untouchedSample");
-        // we test unasignment of this sample from the experiment
-        SamplePE unassignedSample = createSampleWithCode("unassignedSample");
-        // we test if this sample will be assigned to the experiment
-        SamplePE assignedSample = createSampleWithCode("assignedSample");
+        ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
+        ExperimentPE experiment = CommonTestUtils.createExperiment(identifier);
+        experiment.setProperties(Collections.<EntityPropertyPE> emptySet());
+        prepareLoadingExperiment(identifier, experiment);
+        ExperimentUpdatesDTO update = createDefaultUpdateObject(experiment);
+        update.setOriginalSampleCodes(new String[]
+            { "S1" });
+        update.setSampleCodes(new String[]
+            { "S1", "S2" });
+        prepareAddSamplesToExperiment(experiment, false, "S2");
+        ExperimentBO experimentBO = loadExperiment(identifier, experiment);
+
+        experimentBO.update(update);
 
-        final ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
-        final ExperimentPE exp = CommonTestUtils.createExperiment(identifier);
-        exp.setSamples(Arrays.asList(untouchedSample, unassignedSample));
+        context.assertIsSatisfied();
+    }
 
-        prepareLoadExperimentByIdentifier(identifier, exp);
-        prepareTryFindSample(exp.getProject().getSpace(), assignedSample.getCode(), assignedSample);
-        prepareNoDatasetsFound();
-        final ExperimentBO expBO = loadExperiment(identifier, exp);
+    @Test(expectedExceptionsMessageRegExp = "Sample 'S2' is already assigned.*", expectedExceptions = UserFailureException.class)
+    public void testUpdateByAddingAnAlreadyAssignedSample()
+    {
+        ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
+        ExperimentPE experiment = CommonTestUtils.createExperiment(identifier);
+        experiment.setProperties(Collections.<EntityPropertyPE> emptySet());
+        prepareLoadingExperiment(identifier, experiment);
+        ExperimentUpdatesDTO update = createDefaultUpdateObject(experiment);
+        update.setOriginalSampleCodes(new String[]
+            { "S1" });
+        update.setSampleCodes(new String[]
+            { "S1", "S2" });
+        prepareAddSamplesToExperiment(experiment, true, "S2");
+        ExperimentBO experimentBO = loadExperiment(identifier, experiment);
+
+        experimentBO.update(update);
 
-        String[] editedSamples = new String[]
-            { untouchedSample.getCode(), assignedSample.getCode() };
-        expBO.setExperimentSamples(editedSamples);
-        assertEquals(exp, untouchedSample.getExperiment());
+        context.assertIsSatisfied();
     }
 
-    private void prepareNoDatasetsFound()
+    @Test(expectedExceptionsMessageRegExp = "Samples with following codes do not exist "
+            + "in the space 'HOME_GROUP': '\\[S2\\]'\\.", expectedExceptions = UserFailureException.class)
+    public void testUpdateByAddingAnUnknownSample()
     {
+        ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
+        final ExperimentPE experiment = CommonTestUtils.createExperiment(identifier);
+        experiment.setProperties(Collections.<EntityPropertyPE> emptySet());
+        prepareLoadingExperiment(identifier, experiment);
+        ExperimentUpdatesDTO update = createDefaultUpdateObject(experiment);
+        update.setOriginalSampleCodes(new String[]
+            { "S1" });
+        update.setSampleCodes(new String[]
+            { "S1", "S2" });
         context.checking(new Expectations()
             {
                 {
-                    allowing(dataDAO).hasDataSet((with(any(SamplePE.class))));
-                    will(returnValue(false));
-
-                    one(relationshipService).assignSampleToExperiment(
-                            with(any(IAuthSession.class)), with(any(SamplePE.class)),
-                            with(any(ExperimentPE.class)));
-
-                    one(relationshipService).unassignSampleFromExperiment(
-                            with(any(IAuthSession.class)), with(any(SamplePE.class)));
+                    SpacePE space = experiment.getProject().getSpace();
+                    one(sampleDAO).tryFindByCodeAndSpace("S2", space);
                 }
             });
-    }
+        ExperimentBO experimentBO = loadExperiment(identifier, experiment);
 
-    @Test
-    public final void testEditSamplesAddingAssignedSampleFails()
-    {
-        SamplePE assignedSample = createSampleWithCode("assignedSample");
-        assignedSample.setExperiment(createExperiment("anotherExp"));
-
-        final ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
-        final ExperimentPE exp = CommonTestUtils.createExperiment(identifier);
-        assert exp.getSamples().size() == 0 : "no samples expected";
+        experimentBO.update(update);
 
-        prepareLoadExperimentByIdentifier(identifier, exp);
-        prepareTryFindSample(exp.getProject().getSpace(), assignedSample.getCode(), assignedSample);
-
-        final ExperimentBO expBO = loadExperiment(identifier, exp);
-
-        String[] editedSamples = new String[]
-            { assignedSample.getCode() };
-
-        String errorMsg = "Sample 'assignedSample' is already assigned to the experiment";
-        try
-        {
-            expBO.setExperimentSamples(editedSamples);
-        } catch (UserFailureException e)
-        {
-
-            AssertionUtil.assertContains(errorMsg, e.getMessage());
-            return;
-        }
-        fail("exception expected with the error msg: " + errorMsg);
+        context.assertIsSatisfied();
     }
 
     @Test
-    public final void testEditSamplesAssigningUnexistingSampleFails()
+    public void testUpdateByAddingAndRemovingSamples()
     {
-        String unknownSampleCode = "unknownSampleCode";
+        ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
+        ExperimentPE experiment = CommonTestUtils.createExperiment(identifier);
+        experiment.setProperties(Collections.<EntityPropertyPE> emptySet());
+        prepareLoadingExperiment(identifier, experiment);
+        ExperimentUpdatesDTO update = createDefaultUpdateObject(experiment);
+        update.setOriginalSampleCodes(new String[]
+            { "S1", "S2" });
+        update.setSampleCodes(new String[]
+            { "S3", "S2" });
+        prepareRemoveSamplesFromExperiment(experiment, false, "S1");
+        prepareAddSamplesToExperiment(experiment, false, "S3");
+        ExperimentBO experimentBO = loadExperiment(identifier, experiment);
+
+        experimentBO.update(update);
 
-        final ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
-        final ExperimentPE exp = CommonTestUtils.createExperiment(identifier);
+        context.assertIsSatisfied();
+    }
 
-        prepareLoadExperimentByIdentifier(identifier, exp);
-        final ExperimentBO expBO = loadExperiment(identifier, exp);
+    @Test(expectedExceptionsMessageRegExp = ".*datasets.*", expectedExceptions = UserFailureException.class)
+    public void testUpdateByRemovingASampleWithDataSets()
+    {
+        ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
+        ExperimentPE experiment = CommonTestUtils.createExperiment(identifier);
+        experiment.setProperties(Collections.<EntityPropertyPE> emptySet());
+        prepareLoadingExperiment(identifier, experiment);
+        ExperimentUpdatesDTO update = createDefaultUpdateObject(experiment);
+        update.setOriginalSampleCodes(new String[]
+            { "S1", "S2" });
+        update.setSampleCodes(new String[]
+            { "S2" });
+        prepareRemoveSamplesFromExperiment(experiment, true, "S1");
+        ExperimentBO experimentBO = loadExperiment(identifier, experiment);
+
+        experimentBO.update(update);
 
-        prepareTryFindSample(exp.getProject().getSpace(), unknownSampleCode, null);
-        String errorMsg =
-                "Samples with following codes do not exist in the space 'HOME_GROUP': '[unknownSampleCode]'.";
-        try
-        {
-            expBO.setExperimentSamples(new String[]
-                { unknownSampleCode });
-        } catch (UserFailureException e)
-        {
+        context.assertIsSatisfied();
+    }
 
-            assertEquals(errorMsg, e.getMessage());
-            return;
-        }
-        fail("exception expected with the error msg: " + errorMsg);
+    private ExperimentUpdatesDTO createDefaultUpdateObject(ExperimentPE experiment)
+    {
+        ExperimentUpdatesDTO update = new ExperimentUpdatesDTO();
+        update.setExperimentId(new TechId(experiment));
+        update.setProjectIdentifier(new ExperimentIdentifierFactory(experiment.getIdentifier())
+                .createIdentifier());
+        update.setAttachments(Collections.<NewAttachment> emptyList());
+        update.setProperties(Collections.<IEntityProperty> emptyList());
+        return update;
     }
 
-    @Test
-    public final void testEditSamplesUnassigningSampleWithDatasetsFails()
+    private void prepareLoadingExperiment(ExperimentIdentifier identifier,
+            final ExperimentPE experiment)
     {
-        final SamplePE assignedSample = createSampleWithCode("assignedSample");
+        prepareLoadingExperiment(experiment);
+        prepareLoadExperimentByIdentifier(identifier, experiment);
+        context.checking(new Expectations()
+            {
+                {
+                    ProjectPE projectPE = experiment.getProject();
+                    SpacePE space = projectPE.getSpace();
+                    one(projectDAO).tryFindProject(space.getDatabaseInstance().getCode(),
+                            space.getCode(), projectPE.getCode());
+                    will(returnValue(projectPE));
+                }
+            });
+    }
 
-        final ExperimentIdentifier identifier = CommonTestUtils.createExperimentIdentifier();
-        final ExperimentPE exp = CommonTestUtils.createExperiment(identifier);
-        exp.setSamples(Arrays.asList(assignedSample));
+    private void prepareAddSamplesToExperiment(final ExperimentPE experiment,
+            final boolean alreadyAssignedToAnExperiment, final String... sampleCodes)
+    {
+        context.checking(new Expectations()
+            {
+                {
+                    SpacePE space = experiment.getProject().getSpace();
+                    for (String sampleCode : sampleCodes)
+                    {
+                        one(sampleDAO).tryFindByCodeAndSpace(sampleCode, space);
+                        SamplePE sample = createSampleWithCode(sampleCode);
+                        if (alreadyAssignedToAnExperiment)
+                        {
+                            sample.setExperiment(experiment);
+                        }
+                        will(returnValue(sample));
+                        if (alreadyAssignedToAnExperiment)
+                        {
+                            break;
+                        }
+                        one(relationshipService).assignSampleToExperiment(EXAMPLE_SESSION, sample,
+                                experiment);
+                    }
+                }
+            });
+    }
 
-        prepareLoadExperimentByIdentifier(identifier, exp);
-        final ExperimentBO expBO = loadExperiment(identifier, exp);
+    private void prepareRemoveSamplesFromExperiment(final ExperimentPE experiment,
+            final boolean samplesHaveDataSets, final String... sampleCodes)
+    {
         context.checking(new Expectations()
             {
                 {
-                    allowing(dataDAO).hasDataSet(with(assignedSample));
-                    will(returnValue(true));
+                    SpacePE space = experiment.getProject().getSpace();
+                    for (String sampleCode : sampleCodes)
+                    {
+                        one(sampleDAO).tryFindByCodeAndSpace(sampleCode, space);
+                        SamplePE sample = createSampleWithCode(sampleCode);
+                        will(returnValue(sample));
+
+                        one(dataDAO).hasDataSet(sample);
+                        will(returnValue(samplesHaveDataSets));
+                        if (samplesHaveDataSets)
+                        {
+                            break;
+                        }
+                        one(relationshipService).unassignSampleFromExperiment(EXAMPLE_SESSION,
+                                sample);
+                    }
                 }
             });
+    }
 
-        String errorMsg =
-                "Operation cannot be performed, because some datasets have been already produced for the sample 'assignedSample'.";
-        try
-        {
-            expBO.setExperimentSamples(new String[] {}); // remove all samples
-        } catch (UserFailureException e)
-        {
+    private void prepareLoadingExperiment(final ExperimentPE experiment)
+    {
+        context.checking(new Expectations()
+            {
+                {
+                    one(experimentDAO).tryGetByTechId(new TechId(experiment),
+                            ExperimentBO.PROPERTY_TYPES);
+                    will(returnValue(experiment));
 
-            assertEquals(errorMsg, e.getMessage());
-            return;
-        }
-        fail("exception expected with the error msg: " + errorMsg);
+                    one(entityTypeDAO).listEntityTypes();
+                    will(returnValue(Arrays.asList(experiment.getExperimentType())));
+
+                    allowing(entityPropertyTypeDAO).listEntityPropertyTypes(
+                            experiment.getExperimentType());
+                }
+            });
     }
 
     @Test
@@ -881,13 +946,6 @@ public final class ExperimentBOTest extends AbstractBOTest
         context.assertIsSatisfied();
     }
 
-    private static ExperimentPE createExperiment(String code)
-    {
-        ExperimentIdentifier ident =
-                new ExperimentIdentifier(CommonTestUtils.createProjectIdentifier(), code);
-        return CommonTestUtils.createExperiment(ident);
-    }
-
     private ExperimentBO loadExperiment(final ExperimentIdentifier identifier,
             final ExperimentPE exp)
     {
@@ -897,18 +955,6 @@ public final class ExperimentBOTest extends AbstractBOTest
         return expBO;
     }
 
-    private void prepareTryFindSample(final SpacePE group, final String sampleCode,
-            final SamplePE foundSample)
-    {
-        context.checking(new Expectations()
-            {
-                {
-                    one(sampleDAO).tryFindByCodeAndSpace(sampleCode, group);
-                    will(returnValue(foundSample));
-                }
-            });
-    }
-
     private static SamplePE createSampleWithCode(String code)
     {
         SamplePE s = CommonTestUtils.createSample();
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/CommonTestUtils.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/CommonTestUtils.java
index 9514eb31013..bdc3b8dc3d9 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/CommonTestUtils.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/CommonTestUtils.java
@@ -372,6 +372,7 @@ public class CommonTestUtils
         final ExperimentPE exp = new ExperimentPE();
         final ExperimentTypePE expType = new ExperimentTypePE();
         expType.setCode("TEST-EXP-TYPE");
+        expType.setDatabaseInstance(new DatabaseInstancePE());
         exp.setId(42L);
         exp.setExperimentType(expType);
         exp.setCode(ei.getExperimentCode());
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/optimistic_locking/ExperimentOptimisticLockingTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/optimistic_locking/ExperimentOptimisticLockingTest.java
index 3f027e6a971..a781f37df5d 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/optimistic_locking/ExperimentOptimisticLockingTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/optimistic_locking/ExperimentOptimisticLockingTest.java
@@ -87,7 +87,7 @@ public class ExperimentOptimisticLockingTest extends OptimisticLockingTestCase
         genericServer.updateExperiment(sessionToken, updates);
 
         Experiment retrievedExperiment = toolBox.loadExperiment(experiment);
-        checkModifierAndModificationDateOfExperiment(timeIntervalChecker, retrievedExperiment,
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, retrievedExperiment,
                 "test");
         assertEquals("DESCRIPTION: testChangePropertyOfAnExistingExperiment", retrievedExperiment
                 .getProperties().get(0).toString());
@@ -141,7 +141,7 @@ public class ExperimentOptimisticLockingTest extends OptimisticLockingTestCase
 
         Experiment retrievedExperiment =
                 commonServer.getExperimentInfo(systemSessionToken, new TechId(experiment));
-        checkModifierAndModificationDateOfExperiment(timeIntervalChecker, retrievedExperiment,
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, retrievedExperiment,
                 "test");
         assertEquals(toolBox.project2.getIdentifier(), retrievedExperiment.getProject()
                 .getIdentifier());
@@ -172,6 +172,32 @@ public class ExperimentOptimisticLockingTest extends OptimisticLockingTestCase
         }
     }
 
+    @Test
+    public void testAddMetaProject()
+    {
+        Experiment experiment = toolBox.createAndLoadExperiment(1);
+        ExperimentUpdatesDTO updates = new ExperimentUpdatesDTO();
+        updates.setVersion(experiment.getVersion());
+        updates.setExperimentId(new TechId(experiment));
+        updates.setProjectIdentifier(toolBox.createProjectIdentifier(experiment.getProject()
+                .getIdentifier()));
+        updates.setAttachments(ToolBox.NO_ATTACHMENTS);
+        updates.setProperties(experiment.getProperties());
+        updates.setMetaprojectsOrNull(new String[]
+            { "TEST_METAPROJECTS" });
+        String sessionToken = logIntoCommonClientService().getSessionID();
+        assertEquals("", toolBox.renderMetaProjects(toolBox
+                .loadExperiment(sessionToken, experiment).getMetaprojects()));
+
+        genericServer.updateExperiment(sessionToken, updates);
+
+        Experiment loadedExperiment = toolBox.loadExperiment(sessionToken, experiment);
+        assertEquals(experiment.getModifier(), loadedExperiment.getModifier());
+        assertEquals(experiment.getModificationDate(), loadedExperiment.getModificationDate());
+        assertEquals("/test/TEST_METAPROJECTS",
+                toolBox.renderMetaProjects(loadedExperiment.getMetaprojects()));
+    }
+
     @Test
     public void testMoveSampleBetweenExperiments()
     {
@@ -193,14 +219,14 @@ public class ExperimentOptimisticLockingTest extends OptimisticLockingTestCase
         etlService.performEntityOperations(sessionToken, details);
 
         Experiment loadedExperiment1 = toolBox.loadExperiment(experiment1);
-        checkModifierAndModificationDateOfExperiment(timeIntervalChecker, loadedExperiment1,
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, loadedExperiment1,
                 ToolBox.USER_ID);
         List<Sample> samples1 =
                 commonServer.listSamples(systemSessionToken,
                         ListSampleCriteria.createForExperiment(new TechId(loadedExperiment1)));
         assertEquals("[]", samples1.toString());
         Experiment loadedExperiment2 = toolBox.loadExperiment(experiment2);
-        checkModifierAndModificationDateOfExperiment(timeIntervalChecker, loadedExperiment2,
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, loadedExperiment2,
                 ToolBox.USER_ID);
         List<Sample> samples2 =
                 commonServer.listSamples(systemSessionToken,
@@ -209,14 +235,19 @@ public class ExperimentOptimisticLockingTest extends OptimisticLockingTestCase
     }
 
     @Test
-    public void testRemoveSampleFromExperiment()
+    public void testRemoveAndAddSamples()
     {
         Experiment experiment = toolBox.createAndLoadExperiment(1);
-        toolBox.createAndLoadSample(1, experiment);
+        Sample sample1 = toolBox.createAndLoadSample(1, experiment);
+        Sample sample2 = toolBox.createAndLoadSample(2, experiment);
+        Sample sample3 = toolBox.createAndLoadSample(3, null);
         ExperimentUpdatesDTO update = new ExperimentUpdatesDTO();
         update.setVersion(toolBox.loadExperiment(experiment).getVersion());
         update.setExperimentId(new TechId(experiment));
-        update.setSampleCodes(new String[0]);
+        update.setOriginalSampleCodes(new String[]
+            { sample1.getCode(), sample2.getCode() });
+        update.setSampleCodes(new String[]
+            { sample2.getCode(), sample3.getCode() });
         update.setProperties(ToolBox.NO_PROPERTIES);
         update.setAttachments(ToolBox.NO_ATTACHMENTS);
         update.setProjectIdentifier(toolBox.createProjectIdentifier(experiment.getProject()
@@ -227,11 +258,24 @@ public class ExperimentOptimisticLockingTest extends OptimisticLockingTestCase
         genericServer.updateExperiment(sessionToken, update);
 
         Experiment loadedExperiment = toolBox.loadExperiment(experiment);
-        checkModifierAndModificationDateOfExperiment(timeIntervalChecker, loadedExperiment, "test");
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, loadedExperiment,
+                "test");
         List<Sample> samples =
                 commonServer.listSamples(systemSessionToken,
                         ListSampleCriteria.createForExperiment(new TechId(loadedExperiment)));
-        assertEquals("[]", samples.toString());
+        assertEquals("[" + sample2.getCode() + ", " + sample3.getCode() + "]", toolBox
+                .extractCodes(samples).toString());
+        Sample reloadedSample1 = toolBox.loadSample(sample1);
+        assertEquals(null, reloadedSample1.getExperiment());
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, reloadedSample1, "test");
+        Sample reloadedSample2 = toolBox.loadSample(sample2);
+        assertEquals(experiment.getIdentifier(), reloadedSample2.getExperiment().getIdentifier());
+        assertEquals(sample2.getModifier(), reloadedSample2.getModifier());
+        assertEquals(sample2.getModificationDate(), reloadedSample2.getModificationDate());
+        Sample reloadedSample3 = toolBox.loadSample(sample3);
+        assertEquals(experiment.getIdentifier(), reloadedSample3.getExperiment().getIdentifier());
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, reloadedSample3, "test");
+
     }
 
     @Test
@@ -259,22 +303,17 @@ public class ExperimentOptimisticLockingTest extends OptimisticLockingTestCase
         genericServer.updateDataSet(sessionToken, update);
 
         Experiment loadedExperiment1 = toolBox.loadExperiment(exp1);
-        checkModifierAndModificationDateOfExperiment(timeIntervalChecker, loadedExperiment1, "test");
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, loadedExperiment1,
+                "test");
         List<ExternalData> dataSets1 =
                 etlService.listDataSetsByExperimentID(systemSessionToken, new TechId(exp1));
         assertEquals("[]", dataSets1.toString());
         Experiment loadedExperiment2 = toolBox.loadExperiment(exp2);
-        checkModifierAndModificationDateOfExperiment(timeIntervalChecker, loadedExperiment2, "test");
+        toolBox.checkModifierAndModificationDateOfBean(timeIntervalChecker, loadedExperiment2,
+                "test");
         List<ExternalData> dataSets2 =
                 etlService.listDataSetsByExperimentID(systemSessionToken, new TechId(exp2));
         assertEquals(dataSet.getCode(), dataSets2.get(0).getCode());
     }
 
-    private void checkModifierAndModificationDateOfExperiment(
-            TimeIntervalChecker timeIntervalChecker, Experiment experiment, String userId)
-    {
-        assertEquals(userId, experiment.getModifier().getUserId());
-        timeIntervalChecker.assertDateInInterval(experiment.getModificationDate());
-    }
-
 }
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/optimistic_locking/ToolBox.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/optimistic_locking/ToolBox.java
index d63c613253c..e1b547579ac 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/optimistic_locking/ToolBox.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/optimistic_locking/ToolBox.java
@@ -24,6 +24,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
+import ch.systemsx.cisd.common.shared.basic.string.CommaSeparatedListBuilder;
 import ch.systemsx.cisd.openbis.generic.server.util.TimeIntervalChecker;
 import ch.systemsx.cisd.openbis.generic.shared.ICommonServer;
 import ch.systemsx.cisd.openbis.generic.shared.IETLLIMSService;
@@ -40,6 +41,7 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Grantee;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.IEntityProperty;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ListSampleCriteria;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.LocatorType;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Metaproject;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewAttachment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewExperiment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
@@ -294,7 +296,12 @@ public class ToolBox
 
     public Experiment loadExperiment(final IIdentifierHolder experiment)
     {
-        return commonServer.getExperimentInfo(systemSessionToken,
+        return loadExperiment(systemSessionToken, experiment);
+    }
+
+    public Experiment loadExperiment(String sessionToken, final IIdentifierHolder experiment)
+    {
+        return commonServer.getExperimentInfo(sessionToken,
                 ExperimentIdentifierFactory.parse(experiment.getIdentifier()));
     }
 
@@ -410,4 +417,14 @@ public class ToolBox
             // ignored
         }
     }
+
+    public String renderMetaProjects(Collection<Metaproject> metaprojects)
+    {
+        CommaSeparatedListBuilder builder = new CommaSeparatedListBuilder();
+        for (Metaproject metaproject : metaprojects)
+        {
+            builder.append(metaproject.getIdentifier());
+        }
+        return builder.toString();
+    }
 }
-- 
GitLab