From 7cd282c520f8dc0a8118b374f481fc0f4fdba704 Mon Sep 17 00:00:00 2001 From: izabel <izabel> Date: Wed, 18 Mar 2009 08:25:43 +0000 Subject: [PATCH] [LMS-780] check sample consistency + server tests SVN: 10274 --- .../openbis/generic/server/CommonServer.java | 61 +++++++- .../server/business/bo/IExperimentBO.java | 2 +- .../generic/server/CommonServerTest.java | 143 +++++++++++++++--- .../shared/AbstractServerTestCase.java | 10 +- 4 files changed, 182 insertions(+), 34 deletions(-) diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java index 8fbfc5f9d77..605e4b97ef5 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java @@ -648,20 +648,65 @@ public final class CommonServer extends AbstractServer<ICommonServer> implements final Session session = getSessionManager().getSession(sessionToken); if (newProjectIdentifier.equals(identifier) == false) { - final IExternalDataTable externalDataTable = - businessObjectFactory.createExternalDataTable(session); - externalDataTable.loadByExperimentIdentifier(identifier); - if (externalDataTable.getExternalData().size() > 0) - { - throw new UserFailureException( - "Changing the project of experiment containing data sets is not allowed."); - } + checkExternalData(identifier, session); + checkSampleConsistency(identifier, newProjectIdentifier, session); } + final IExperimentBO experimentBO = businessObjectFactory.createExperimentBO(session); experimentBO.edit(identifier, properties, attachments, newProjectIdentifier); experimentBO.save(); } + private void checkExternalData(ExperimentIdentifier identifier, final Session session) + { + final IExternalDataTable externalDataTable = + businessObjectFactory.createExternalDataTable(session); + externalDataTable.loadByExperimentIdentifier(identifier); + if (externalDataTable.getExternalData().size() > 0) + { + throw new UserFailureException( + "Changing the project of experiment containing data sets is not allowed."); + } + } + + private void checkSampleConsistency(ExperimentIdentifier identifier, + ProjectIdentifier newProjectIdentifier, final Session session) + { + ListSampleCriteriaDTO criteria = + ListSampleCriteriaDTO.createExperimentIdentifier(identifier); + final ISampleTable sampleTable = businessObjectFactory.createSampleTable(session); + sampleTable.loadSamplesByCriteria(criteria); + final List<SamplePE> samples = sampleTable.getSamples(); + if (samples.size() > 0) + { + checkExperimentGroupMatches(samples.get(0).getSampleIdentifier(), + newProjectIdentifier); + } + } + + private void checkExperimentGroupMatches(SampleIdentifier sampleIdentifier, + ProjectIdentifier newProjectIdentifier) + { + assert sampleIdentifier != null : "Sample identifier not specified"; + assert newProjectIdentifier != null : "Project identifier not specified"; + final GroupIdentifier sampleGroup = sampleIdentifier.getGroupLevel(); + if (sampleGroup == null) + { + throw new UserFailureException( + "Inconsistency detected: shared sample found in experiment."); + } + if (sampleGroup.getDatabaseInstanceCode().equals( + newProjectIdentifier.getDatabaseInstanceCode()) == false + || sampleGroup.getGroupCode().equals(newProjectIdentifier.getGroupCode()) == false) + { + throw new UserFailureException( + String + .format( + "Project cannot be changed to '%s' because experiment containes samples from group '%s'.", + newProjectIdentifier, sampleGroup)); + } + } + public void editMaterial(String sessionToken, MaterialIdentifier identifier, List<MaterialProperty> properties) { diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/IExperimentBO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/IExperimentBO.java index e9154b16db9..0cbef8d614a 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/IExperimentBO.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/IExperimentBO.java @@ -65,5 +65,5 @@ public interface IExperimentBO extends IBusinessObject * Changes given experiment. Currently allowed changes: properties. */ public void edit(ExperimentIdentifier identifier, List<ExperimentProperty> properties, - List<AttachmentPE> attachments, ProjectIdentifier newProjectIdentifierOrNull); + List<AttachmentPE> attachments, ProjectIdentifier newProjectIdentifier); } diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/CommonServerTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/CommonServerTest.java index bce3698b2af..0ef580f3401 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/CommonServerTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/CommonServerTest.java @@ -31,8 +31,13 @@ import ch.systemsx.cisd.openbis.generic.shared.AbstractServerTestCase; import ch.systemsx.cisd.openbis.generic.shared.CommonTestUtils; import ch.systemsx.cisd.openbis.generic.shared.ICommonServer; import ch.systemsx.cisd.openbis.generic.shared.IDataStoreService; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ExperimentProperty; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.MaterialIdentifier; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.MaterialProperty; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.PropertyType; +import ch.systemsx.cisd.openbis.generic.shared.basic.dto.SampleProperty; import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Vocabulary; +import ch.systemsx.cisd.openbis.generic.shared.dto.AttachmentPE; import ch.systemsx.cisd.openbis.generic.shared.dto.DataStoreServerSession; import ch.systemsx.cisd.openbis.generic.shared.dto.DataTypePE; import ch.systemsx.cisd.openbis.generic.shared.dto.EntityTypePE; @@ -50,8 +55,10 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.Session; import ch.systemsx.cisd.openbis.generic.shared.dto.VocabularyPE; import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.DatabaseInstanceIdentifier; import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifier; +import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.GroupIdentifier; import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ProjectIdentifier; import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifier; +import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleOwnerIdentifier; import ch.systemsx.cisd.openbis.generic.shared.dto.properties.EntityKind; /** @@ -61,14 +68,30 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.properties.EntityKind; */ public final class CommonServerTest extends AbstractServerTestCase { + private static final String MATERIAL_TYPE_1 = "MATERIAL-TYPE-1"; + + private static final String MATERIAL_1 = "MATERIAL-1"; + + private static final String SAMPLE_1 = "SAMPLE-1"; + + private static final String EXP_1 = "EXP-1"; + + private static final String PROJECT_1 = "PROJECT-1"; + + private static final String GROUP_1 = "GROUP-1"; + + private static final String DATABASE_1 = "DATABASE-1"; + private ICommonBusinessObjectFactory commonBusinessObjectFactory; + private DataStoreServerSessionManager dssSessionManager; + private IDataStoreService dataStoreService; private final ICommonServer createServer() { - return new CommonServer(authenticationService, sessionManager, - dssSessionManager, daoFactory, commonBusinessObjectFactory); + return new CommonServer(authenticationService, sessionManager, dssSessionManager, + daoFactory, commonBusinessObjectFactory); } private final static PersonPE createSystemUser() @@ -489,8 +512,7 @@ public final class CommonServerTest extends AbstractServerTestCase will(returnValue(new ArrayList<ExperimentPE>())); } }); - createServer() - .listExperiments(SESSION_TOKEN, experimentType, projectIdentifier); + createServer().listExperiments(SESSION_TOKEN, experimentType, projectIdentifier); context.assertIsSatisfied(); } @@ -709,7 +731,7 @@ public final class CommonServerTest extends AbstractServerTestCase will(returnValue(d1)); } }); - + try { createServer().deleteDataSets(SESSION_TOKEN, Arrays.asList(d1.getCode()), ""); @@ -724,12 +746,13 @@ public final class CommonServerTest extends AbstractServerTestCase context.assertIsSatisfied(); } - + @Test public void testDeleteDataSetsButOneDataSetIsUnknown() { prepareGetSession(); - dssSessionManager.registerDataStoreServer(new DataStoreServerSession("url", dataStoreService)); + dssSessionManager.registerDataStoreServer(new DataStoreServerSession("url", + dataStoreService)); final ExternalDataPE d1 = createDataSet("d1"); final ExternalDataPE d2 = createDataSet("d2"); context.checking(new Expectations() @@ -745,30 +768,104 @@ public final class CommonServerTest extends AbstractServerTestCase one(dataStoreService).getKnownDataSets(with(any(String.class)), with(equal(locations))); will(returnValue(Arrays.asList(d1.getLocation()))); - + } }); - + try { - createServer().deleteDataSets(SESSION_TOKEN, Arrays.asList(d1.getCode(), d2.getCode()), ""); + createServer().deleteDataSets(SESSION_TOKEN, Arrays.asList(d1.getCode(), d2.getCode()), + ""); fail("UserFailureException expected"); } catch (UserFailureException e) { assertEquals( "The following data sets are unknown by any registered Data Store Server. " - + "May be the responsible Data Store Server is not running.\n[d2]", e - .getMessage()); + + "May be the responsible Data Store Server is not running.\n[d2]", e + .getMessage()); } - + + context.assertIsSatisfied(); + } + + @Test + public void testEditMaterialNothingChanged() throws Exception + { + final MaterialIdentifier identifier = + new MaterialIdentifier(MATERIAL_1, MATERIAL_TYPE_1); + final List<MaterialProperty> properties = new ArrayList<MaterialProperty>(); + prepareGetSession(); + context.checking(new Expectations() + { + { + one(commonBusinessObjectFactory).createMaterialBO(SESSION); + will(returnValue(materialBO)); + + one(materialBO).edit(identifier, properties); + one(materialBO).save(); + + } + }); + createServer().editMaterial(SESSION_TOKEN, identifier, properties); + context.assertIsSatisfied(); + } + + @Test + public void testEditSampleNothingChanged() throws Exception + { + final SampleIdentifier identifier = + SampleIdentifier.createOwnedBy(new SampleOwnerIdentifier(new GroupIdentifier( + DATABASE_1, GROUP_1)), SAMPLE_1); + final List<SampleProperty> properties = new ArrayList<SampleProperty>(); + prepareGetSession(); + context.checking(new Expectations() + { + { + one(commonBusinessObjectFactory).createSampleBO(SESSION); + will(returnValue(sampleBO)); + + one(sampleBO).edit(identifier, properties); + one(sampleBO).save(); + + } + }); + createServer().editSample(SESSION_TOKEN, identifier, properties); + context.assertIsSatisfied(); + } + + @Test + public void testEditExperimentNothingChanged() throws Exception + { + final ExperimentIdentifier identifier = + new ExperimentIdentifier(DATABASE_1, GROUP_1, PROJECT_1, EXP_1); + final List<ExperimentProperty> properties = new ArrayList<ExperimentProperty>(); + final List<AttachmentPE> attachments = new ArrayList<AttachmentPE>(); + final ProjectIdentifier newProjectIdentifier = + new ProjectIdentifier(DATABASE_1, GROUP_1, PROJECT_1); + prepareGetSession(); + context.checking(new Expectations() + { + { + one(commonBusinessObjectFactory).createExperimentBO(SESSION); + will(returnValue(experimentBO)); + + one(experimentBO).edit(identifier, properties, attachments, + newProjectIdentifier); + one(experimentBO).save(); + + } + }); + createServer().editExperiment(SESSION_TOKEN, identifier, properties, attachments, + newProjectIdentifier); context.assertIsSatisfied(); } - - @Test + + @Test public void testDeleteDataSets() { prepareGetSession(); - dssSessionManager.registerDataStoreServer(new DataStoreServerSession("url", dataStoreService)); + dssSessionManager.registerDataStoreServer(new DataStoreServerSession("url", + dataStoreService)); final ExternalDataPE d1 = createDataSet("d1"); final ExternalDataPE d2 = createDataSet("d2"); context.checking(new Expectations() @@ -784,18 +881,20 @@ public final class CommonServerTest extends AbstractServerTestCase one(dataStoreService).getKnownDataSets(with(any(String.class)), with(equal(locations))); will(returnValue(locations)); - + one(externalDataDAO).markAsDeleted(d1, SESSION.tryGetPerson(), "reason"); one(externalDataDAO).markAsDeleted(d2, SESSION.tryGetPerson(), "reason"); - one(dataStoreService).deleteDataSets(with(any(String.class)), with(equal(locations))); + one(dataStoreService).deleteDataSets(with(any(String.class)), + with(equal(locations))); } }); - - createServer().deleteDataSets(SESSION_TOKEN, Arrays.asList(d1.getCode(), d2.getCode()), "reason"); - + + createServer().deleteDataSets(SESSION_TOKEN, Arrays.asList(d1.getCode(), d2.getCode()), + "reason"); + context.assertIsSatisfied(); } - + private ExternalDataPE createDataSet(String code) { ExternalDataPE data = new ExternalDataPE(); diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/AbstractServerTestCase.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/AbstractServerTestCase.java index e0d43d80fb9..0e5f2083b1f 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/AbstractServerTestCase.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/AbstractServerTestCase.java @@ -34,6 +34,7 @@ import ch.systemsx.cisd.openbis.generic.server.business.bo.IExperimentTable; import ch.systemsx.cisd.openbis.generic.server.business.bo.IExternalDataBO; import ch.systemsx.cisd.openbis.generic.server.business.bo.IExternalDataTable; import ch.systemsx.cisd.openbis.generic.server.business.bo.IGroupBO; +import ch.systemsx.cisd.openbis.generic.server.business.bo.IMaterialBO; import ch.systemsx.cisd.openbis.generic.server.business.bo.IMaterialTable; import ch.systemsx.cisd.openbis.generic.server.business.bo.IProcedureBO; import ch.systemsx.cisd.openbis.generic.server.business.bo.IPropertyTypeBO; @@ -70,9 +71,9 @@ public abstract class AbstractServerTestCase extends AssertJUnit { protected static final Principal PRINCIPAL = new Principal(CommonTestUtils.USER_ID, "john", "doe", "j@d"); - + protected static final String SESSION_TOKEN = "session-token"; - + protected static final Session SESSION = new Session(CommonTestUtils.USER_ID, SESSION_TOKEN, PRINCIPAL, "remote-host", 1); @@ -104,6 +105,8 @@ public abstract class AbstractServerTestCase extends AssertJUnit protected ISampleBO sampleBO; + protected IMaterialBO materialBO; + protected IExternalDataTable externalDataTable; protected IExperimentTable experimentTable; @@ -162,6 +165,7 @@ public abstract class AbstractServerTestCase extends AssertJUnit // BO groupBO = context.mock(IGroupBO.class); sampleBO = context.mock(ISampleBO.class); + materialBO = context.mock(IMaterialBO.class); experimentBO = context.mock(IExperimentBO.class); propertyTypeBO = context.mock(IPropertyTypeBO.class); vocabularyBO = context.mock(IVocabularyBO.class); @@ -207,7 +211,7 @@ public abstract class AbstractServerTestCase extends AssertJUnit // Otherwise one do not known which test failed. context.assertIsSatisfied(); } - + protected Session createSession() { return new Session(CommonTestUtils.USER_ID, SESSION_TOKEN, PRINCIPAL, "remote-host", 2); -- GitLab