From 1f1684f4c5ea138160299fc37f0151a8985eefb3 Mon Sep 17 00:00:00 2001
From: tpylak <tpylak>
Date: Wed, 31 Aug 2011 14:12:25 +0000
Subject: [PATCH] LMS-2460 fix: allow to update contained samples
SVN: 22747
---
.../bo/AbstractSampleBusinessObject.java | 12 +--
.../generic/shared/basic/dto/NewSample.java | 29 +++--
.../shared/basic/dto/UpdatedSample.java | 2 +-
.../dto/identifier/SampleIdentifier.java | 2 +-
.../identifier/SampleIdentifierFactory.java | 16 +++
.../plugin/generic/server/GenericServer.java | 22 ++--
.../server/SampleRegisterOrUpdateUtil.java | 102 +++++++++++++++---
.../generic/server/GenericServerTest.java | 2 +-
8 files changed, 144 insertions(+), 43 deletions(-)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractSampleBusinessObject.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractSampleBusinessObject.java
index 14faaa33fbe..236de30a602 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractSampleBusinessObject.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/AbstractSampleBusinessObject.java
@@ -109,9 +109,7 @@ abstract class AbstractSampleBusinessObject extends AbstractSampleIdentifierBusi
Map<String, ExperimentPE> experimentCacheOrNull, PersonPE registratorOrNull)
throws UserFailureException
{
- final SampleIdentifier sampleIdentifier =
- SampleIdentifierFactory.parse(newSample.getIdentifier(),
- newSample.getSpaceIdentifier());
+ final SampleIdentifier sampleIdentifier = SampleIdentifierFactory.parse(newSample);
SampleOwner sampleOwner = getSampleOwner(sampleOwnerCacheOrNull, sampleIdentifier);
SampleTypePE sampleTypePE =
(sampleTypeCacheOrNull != null) ? sampleTypeCacheOrNull.get(newSample
@@ -127,7 +125,7 @@ abstract class AbstractSampleBusinessObject extends AbstractSampleIdentifierBusi
String experimentIdentifier = newSample.getExperimentIdentifier();
ExperimentPE experimentPE =
tryFindExperiment(experimentCacheOrNull, experimentIdentifier,
- newSample.getSpaceIdentifier());
+ newSample.getDefaultSpaceIdentifier());
final SamplePE samplePE = new SamplePE();
samplePE.setExperiment(experimentPE);
samplePE.setCode(sampleIdentifier.getSampleSubCode());
@@ -139,11 +137,11 @@ abstract class AbstractSampleBusinessObject extends AbstractSampleIdentifierBusi
defineSampleProperties(samplePE, newSample.getProperties());
String containerIdentifier = newSample.getContainerIdentifier();
setContainer(sampleIdentifier, samplePE, containerIdentifier,
- newSample.getSpaceIdentifier());
+ newSample.getDefaultSpaceIdentifier());
if (newSample.getParentsOrNull() != null)
{
final String[] parents = newSample.getParentsOrNull();
- setParents(samplePE, parents, newSample.getSpaceIdentifier());
+ setParents(samplePE, parents, newSample.getDefaultSpaceIdentifier());
}
samplePE.setPermId(getOrCreatePermID(newSample));
return samplePE;
@@ -575,7 +573,7 @@ abstract class AbstractSampleBusinessObject extends AbstractSampleIdentifierBusi
for (SampleIdentifier sampleIdentifier : sampleIdentifiers)
{
final SampleOwner sampleOwner = getSampleOwner(sampleOwnerCache, sampleIdentifier);
- final String containerCodeOrNull = sampleIdentifier.getContainerCodeOrNull();
+ final String containerCodeOrNull = sampleIdentifier.tryGetContainerCode();
final SampleOwnerWithContainer owner =
new SampleOwnerWithContainer(sampleOwner, containerCodeOrNull);
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/NewSample.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/NewSample.java
index 62eb75fc78f..395f4793293 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/NewSample.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/NewSample.java
@@ -45,6 +45,8 @@ public class NewSample extends Identifier<NewSample> implements Comparable<NewSa
public static final String CONTAINER = "container";
+ public static final String DEFAULT_CONTAINER = "default_container";
+
public static final String PARENT = "parent";
public static final String PARENTS = "parents";
@@ -74,7 +76,7 @@ public class NewSample extends Identifier<NewSample> implements Comparable<NewSa
/***
* The space code for this row home space
*/
- private String spaceIdentifier;
+ private String defaultSpaceIdentifier;
private IEntityProperty[] properties = IEntityProperty.EMPTY_ARRAY;
@@ -115,7 +117,7 @@ public class NewSample extends Identifier<NewSample> implements Comparable<NewSa
this(identifier, sampleType, containerIdentifier);
this.parentsOrNull = parentsOrNull;
this.experimentIdentifier = experimentIdentifier;
- this.spaceIdentifier = spaceCode;
+ this.defaultSpaceIdentifier = spaceCode;
this.properties = properties;
this.attachments = attachments;
}
@@ -202,6 +204,17 @@ public class NewSample extends Identifier<NewSample> implements Comparable<NewSa
this.containerIdentifier = container;
}
+ public final String getDefaultContainerIdentifier()
+ {
+ return containerIdentifier;
+ }
+
+ @BeanProperty(label = DEFAULT_CONTAINER, optional = true)
+ public final void setDefaultContainerIdentifier(final String defaultContainer)
+ {
+ this.containerIdentifier = defaultContainer;
+ }
+
public String getExperimentIdentifier()
{
return experimentIdentifier;
@@ -213,15 +226,15 @@ public class NewSample extends Identifier<NewSample> implements Comparable<NewSa
this.experimentIdentifier = experimentIdentifier;
}
- public String getSpaceIdentifier()
+ public String getDefaultSpaceIdentifier()
{
- return spaceIdentifier;
+ return defaultSpaceIdentifier;
}
@BeanProperty(label = SPACE, optional = true)
- public void setSpaceIdentifier(String spaceIdentifier)
+ public void setDefaultSpaceIdentifier(String spaceIdentifier)
{
- this.spaceIdentifier = spaceIdentifier;
+ this.defaultSpaceIdentifier = spaceIdentifier;
}
public final IEntityProperty[] getProperties()
@@ -269,10 +282,10 @@ public class NewSample extends Identifier<NewSample> implements Comparable<NewSa
}
final NewSample that = (NewSample) obj;
final String thisCombinedIdentifier =
- StringUtils.emptyIfNull(this.getSpaceIdentifier()) + this.getIdentifier()
+ StringUtils.emptyIfNull(this.getDefaultSpaceIdentifier()) + this.getIdentifier()
+ this.getContainerIdentifier();
final String thatCombinedIdentifier =
- StringUtils.emptyIfNull(this.getSpaceIdentifier()) + that.getIdentifier()
+ StringUtils.emptyIfNull(this.getDefaultSpaceIdentifier()) + that.getIdentifier()
+ that.getContainerIdentifier();
return thisCombinedIdentifier.equals(thatCombinedIdentifier);
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/UpdatedSample.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/UpdatedSample.java
index ac3c3dc3fdc..747daae8e9b 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/UpdatedSample.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/UpdatedSample.java
@@ -43,7 +43,7 @@ public final class UpdatedSample extends NewSample
{
super(newSample.getIdentifier(), newSample.getSampleType(), newSample
.getContainerIdentifier(), newSample.getParentsOrNull(), newSample
- .getExperimentIdentifier(), newSample.getSpaceIdentifier(), newSample
+ .getExperimentIdentifier(), newSample.getDefaultSpaceIdentifier(), newSample
.getProperties(), newSample.getAttachments());
this.batchUpdateDetails = batchUpdateDetails;
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifier.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifier.java
index e4913fb6358..60ffc9aae96 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifier.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifier.java
@@ -96,7 +96,7 @@ public class SampleIdentifier extends SampleOwnerIdentifier
return sampleSubCode;
}
- public String getContainerCodeOrNull()
+ public String tryGetContainerCode()
{
return containerCodeOrNull;
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifierFactory.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifierFactory.java
index 92cfe0ee2e1..f9061855806 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifierFactory.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/identifier/SampleIdentifierFactory.java
@@ -21,6 +21,7 @@ import static ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdent
import org.apache.commons.lang.ArrayUtils;
import ch.systemsx.cisd.common.exceptions.UserFailureException;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.DatabaseInstanceIdentifier.Constants;
/**
@@ -41,6 +42,21 @@ public final class SampleIdentifierFactory extends AbstractIdentifierFactory
return new SampleIdentifierFactory(textToParse).createIdentifier(null);
}
+ public static final SampleIdentifier parse(final NewSample sample) throws UserFailureException
+ {
+ SampleIdentifierFactory factory = new SampleIdentifierFactory(sample.getIdentifier());
+ String defaultSpace = sample.getDefaultSpaceIdentifier();
+ SampleIdentifier identifier = factory.createIdentifier(defaultSpace);
+ // if the container for the new sample is not specified then use the default (if provided)
+ String defaultContainer = sample.getDefaultContainerIdentifier();
+ if (identifier.tryGetContainerCode() == null && defaultContainer != null)
+ {
+ SampleIdentifier defaultContainerIdentifier = parse(defaultContainer, defaultSpace);
+ identifier.addContainerCode(defaultContainerIdentifier.getSampleSubCode());
+ }
+ return identifier;
+ }
+
public static final SampleIdentifier parse(final String textToParse, final String defaultSpace)
throws UserFailureException
{
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/GenericServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/GenericServer.java
index 7d62578ab4c..4b886955904 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/GenericServer.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/GenericServer.java
@@ -293,15 +293,20 @@ public final class GenericServer extends AbstractServer<IGenericServer> implemen
updateSamples(session, new NewSamplesWithTypes(sampleType, samplesToUpdate));
}
+ // TODO 2011-08-31, Tomasz Pylak: remove existing hacks
+ // 1. replaces contained samples with their containers if the container is
+ // specified in the contained sample identifier. This should be replaced by a flag not to
+ // update contained samples when the siRNA library is uploaded.
+ // 2. matches samples only by the code, so it can return match more samples if the sample
+ // with the same code exists in a different space. It does not hurt later on (matching is
+ // done by the identifier and additional samples are ignored), but is inefficient.
private List<Sample> fetchExistingSamples(List<NewSample> newSamples)
{
List<Sample> existingSamples = new ArrayList<Sample>();
// add non-contained samples codes
List<String> codes = SampleRegisterOrUpdateUtil.extractCodes(newSamples, false);
- // NOTE 2011-08-17, Tomasz Pylak: this code never updates contained samples,
- // they can be only registered (if they did not exist).
- // So only containers can be updated.
+ // NOTE 2011-08-17, Tomasz Pylak: this code never updates contained samples!
List<Sample> list =
sampleLister.list(SampleRegisterOrUpdateUtil
.createListSamplesByCodeCriteria(codes));
@@ -420,8 +425,7 @@ public final class GenericServer extends AbstractServer<IGenericServer> implemen
for (NewSample updatedSample : updatedSamples)
{
final SampleIdentifier oldSampleIdentifier =
- SampleIdentifierFactory.parse(updatedSample.getIdentifier(),
- updatedSample.getSpaceIdentifier());
+ SampleIdentifierFactory.parse(updatedSample);
final List<IEntityProperty> properties = Arrays.asList(updatedSample.getProperties());
final ExperimentIdentifier experimentIdentifierOrNull;
final SampleIdentifier newSampleIdentifier;
@@ -430,7 +434,8 @@ public final class GenericServer extends AbstractServer<IGenericServer> implemen
// experiment is provided - new sample identifier takes experiment space
experimentIdentifierOrNull =
new ExperimentIdentifierFactory(updatedSample.getExperimentIdentifier())
- .createIdentifier(updatedSample.getSpaceIdentifier());
+ .createIdentifier(updatedSample.getDefaultSpaceIdentifier());
+ // TODO 2011-08-31, Tomasz Pylak: container is ignored, what does it break?
newSampleIdentifier =
new SampleIdentifier(new GroupIdentifier(
experimentIdentifierOrNull.getDatabaseInstanceCode(),
@@ -447,7 +452,7 @@ public final class GenericServer extends AbstractServer<IGenericServer> implemen
final SampleBatchUpdateDetails batchUpdateDetails =
createBatchUpdateDetails(updatedSample);
- samples.add(new SampleBatchUpdatesDTO(updatedSample.getSpaceIdentifier(),
+ samples.add(new SampleBatchUpdatesDTO(updatedSample.getDefaultSpaceIdentifier(),
oldSampleIdentifier, properties, experimentIdentifierOrNull,
newSampleIdentifier, containerIdentifierOrNull, parentsOrNull,
batchUpdateDetails));
@@ -500,7 +505,8 @@ public final class GenericServer extends AbstractServer<IGenericServer> implemen
List<NewSample> newEntities = newSamplesWithType.getNewEntities();
for (NewSample newSample : newEntities)
{
- newSample.setSpaceIdentifier(new SpaceIdentifier(experimentSpace).toString());
+ newSample.setDefaultSpaceIdentifier(new SpaceIdentifier(experimentSpace)
+ .toString());
}
}
registerSamples(sessionToken, newSamples);
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/SampleRegisterOrUpdateUtil.java b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/SampleRegisterOrUpdateUtil.java
index 2f2429abc41..77ee89295f4 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/SampleRegisterOrUpdateUtil.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/generic/server/SampleRegisterOrUpdateUtil.java
@@ -25,6 +25,7 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Sample;
import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifier;
import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifierFactory;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifier;
/**
* Utility class for sample update or registration.
@@ -47,7 +48,7 @@ public class SampleRegisterOrUpdateUtil
{
for (Sample es : existingSamples)
{
- if (isMatching(ns.getIdentifier(), es.getIdentifier()))
+ if (isMatching(ns, es))
{
samplesToUpdate.add(ns);
}
@@ -56,33 +57,100 @@ public class SampleRegisterOrUpdateUtil
return samplesToUpdate;
}
- /**
- * Creates {@link ListOrSearchSampleCriteria} narrowing listing result to samples given codes.
- */
- static ListOrSearchSampleCriteria createListSamplesByCodeCriteria(List<String> codes)
+ private static boolean isMatching(NewSample newSample, Sample existingSample)
{
- String[] codesAsArray = codes.toArray(new String[0]);
- ListOrSearchSampleCriteria criteria = new ListOrSearchSampleCriteria(codesAsArray, false);
- return criteria;
+ SampleIdentifier newSampleIdentifier = SampleIdentifierFactory.parse(newSample);
+ if (isMatchingIgnoringContainer(existingSample, newSampleIdentifier) == false)
+ {
+ return false;
+ }
+
+ // is container matching?
+ SampleIdentifier containerIdentifier =
+ tryCreateContainerIdentifier(newSample, newSampleIdentifier);
+ Sample containerSample = existingSample.getContainer();
+ return isMatchingIgnoringContainer(containerSample, containerIdentifier);
}
- /**
- * Checks if given identifiers are matching (db instance is not considered).
- */
- private static boolean isMatching(String i1, String i2)
+ private static SampleIdentifier tryCreateContainerIdentifier(NewSample newSample,
+ SampleIdentifier newSampleIdentifier)
{
- if (i1 != null && i2 != null)
+ String newSampleContainerCode = newSampleIdentifier.tryGetContainerCode();
+ String newSampleContainerSpace = newSample.getDefaultSpaceIdentifier();
+ if (newSampleContainerCode == null && newSample.getContainerIdentifier() != null)
{
- return normalize(i1).equals(normalize(i2));
+ SampleIdentifier newSampleContainerIdentifier =
+ SampleIdentifierFactory.parse(newSample.getContainerIdentifier(),
+ newSample.getDefaultSpaceIdentifier());
+ newSampleContainerCode = newSampleContainerIdentifier.getSampleSubCode();
+ newSampleContainerSpace = tryGetSpaceCode(newSampleContainerIdentifier);
+ }
+ if (newSampleContainerCode == null)
+ {
+ return null;
} else
{
- return i1 == i2;
+ return new SampleIdentifier(new SpaceIdentifier(newSampleContainerSpace),
+ newSampleContainerCode);
}
}
- private static String normalize(String id)
+ private static boolean isMatchingIgnoringContainer(Sample existingSample,
+ SampleIdentifier newSampleIdentifier)
{
- return dropDatabaseInstance(id).toUpperCase();
+ if (newSampleIdentifier == null || existingSample == null)
+ {
+ if (newSampleIdentifier != null || existingSample != null)
+ {
+ return false; // only one is null
+ }
+ }
+ if (existingSample == null)
+ {
+ assert newSampleIdentifier == null : "newSampleIdentifier is not null";
+ return true;
+ }
+ assert newSampleIdentifier != null : "newSampleIdentifier is null";
+
+ String newSampleSpace = tryGetSpaceCode(newSampleIdentifier);
+ String existingSampleSpace = existingSample.getSpace().getCode();
+ if (existingSampleSpace.equalsIgnoreCase(newSampleSpace) == false)
+ {
+ return false;
+ }
+
+ String newSampleSubCode = newSampleIdentifier.getSampleSubCode();
+ if (existingSample.getSubCode().equalsIgnoreCase(newSampleSubCode) == false)
+ {
+ return false;
+ }
+ return true;
+ }
+
+ private static String tryGetSpaceCode(SampleIdentifier sampleIdentifier)
+ {
+ if (sampleIdentifier.isSpaceLevel())
+ {
+ String space = sampleIdentifier.getSpaceLevel().getSpaceCode();
+ if (space.startsWith(INSTANCE_SEPARATOR))
+ {
+ space = space.substring(1);
+ }
+ return space;
+ } else
+ {
+ return null;
+ }
+ }
+
+ /**
+ * Creates {@link ListOrSearchSampleCriteria} narrowing listing result to samples given codes.
+ */
+ static ListOrSearchSampleCriteria createListSamplesByCodeCriteria(List<String> codes)
+ {
+ String[] codesAsArray = codes.toArray(new String[0]);
+ ListOrSearchSampleCriteria criteria = new ListOrSearchSampleCriteria(codesAsArray, false);
+ return criteria;
}
private static String dropDatabaseInstance(String id)
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/plugin/generic/server/GenericServerTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/plugin/generic/server/GenericServerTest.java
index db19c5748b4..1bacf4e7439 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/plugin/generic/server/GenericServerTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/plugin/generic/server/GenericServerTest.java
@@ -493,7 +493,7 @@ public final class GenericServerTest extends AbstractServerTestCase
createServer().registerExperiment(SESSION_TOKEN, newExperiment,
Collections.<NewAttachment> emptyList());
- assertEquals("/" + spaceCode, newSample1.getSpaceIdentifier());
+ assertEquals("/" + spaceCode, newSample1.getDefaultSpaceIdentifier());
context.assertIsSatisfied();
}
--
GitLab