From ac9c7342eeedebd6e1888551754ecd568df96034 Mon Sep 17 00:00:00 2001
From: pkupczyk <pkupczyk>
Date: Tue, 11 Dec 2012 13:26:27 +0000
Subject: [PATCH] SP-436 / BIS-219 : Improve entity adaptors for validation
scripts & SP-434 / BIS-219: Performance improvements for requesting entities
in validation / dynamic properties scripts: - properties are now lazy loaded
- performance of fetching properties has been improved (over 20 times faster)
- withProperties flag hasn't been introduced as EAGER loading of properties
together with samples is slower than loading them lazily in a separate query
SVN: 27909
---
.../calculator/AbstractEntityAdaptor.java | 69 ++++++++++++++-----
.../calculator/ExperimentAdaptor.java | 9 +--
.../calculator/ExternalDataAdaptor.java | 3 +-
.../calculator/MaterialAdaptor.java | 3 +-
.../calculator/SampleAdaptor.java | 3 +-
.../openbis/generic/shared/dto/DataPE.java | 3 +-
.../generic/shared/dto/ExperimentPE.java | 3 +-
.../generic/shared/dto/MaterialPE.java | 6 +-
.../openbis/generic/shared/dto/SamplePE.java | 3 +-
.../DynamicPropertyEvaluatorTest.java | 10 +--
.../DynamicPropertyCalculatorTest.java | 14 +---
11 files changed, 71 insertions(+), 55 deletions(-)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/AbstractEntityAdaptor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/AbstractEntityAdaptor.java
index c4ef867c9bb..7c8c5e4df7b 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/AbstractEntityAdaptor.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/AbstractEntityAdaptor.java
@@ -51,34 +51,48 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.hibernate.SearchFieldConstant
*/
public class AbstractEntityAdaptor implements IEntityAdaptor
{
- protected final Map<String, IEntityPropertyAdaptor> propertiesByCode =
- new HashMap<String, IEntityPropertyAdaptor>();
-
private final String code;
+ protected final IEntityPropertiesHolder propertiesHolder;
+
+ protected Map<String, IEntityPropertyAdaptor> propertiesByCode;
+
protected final IDynamicPropertyEvaluator evaluator;
protected static String ENTITY_TYPE_CODE_FIELD = SearchFieldConstants.PREFIX_ENTITY_TYPE
+ SearchFieldConstants.CODE;
- public AbstractEntityAdaptor(String code)
+ public AbstractEntityAdaptor(String code, Collection<IEntityPropertyAdaptor> properties)
{
- this(code, null);
+ this.code = code;
+ this.propertiesHolder = null;
+ this.evaluator = null;
+ initPropertiesMap(properties);
}
- public AbstractEntityAdaptor(String code, IDynamicPropertyEvaluator evaluator)
+ public AbstractEntityAdaptor(IEntityPropertiesHolder propertiesHolder,
+ IDynamicPropertyEvaluator propertyEvaluator)
{
- this.code = code;
- this.evaluator = evaluator;
+ this.code = propertiesHolder.getCode();
+ this.propertiesHolder = propertiesHolder;
+ this.evaluator = propertyEvaluator;
}
- protected void initProperties(IEntityPropertiesHolder propertiesHolder)
+ private void initPropertiesMap()
{
+ if (propertiesHolder == null)
+ {
+ throw new IllegalStateException(
+ "Couldn't init properties if the properties holder has not been set");
+ }
if (evaluator == null)
{
throw new IllegalStateException(
- "Couldn't init properties if the evaluator has not been set");
+ "Couldn't init properties if the property evaluator has not been set");
}
+
+ propertiesByCode = new HashMap<String, IEntityPropertyAdaptor>();
+
for (EntityPropertyPE property : propertiesHolder.getProperties())
{
EntityTypePropertyTypePE etpt = property.getEntityTypePropertyType();
@@ -115,9 +129,32 @@ public class AbstractEntityAdaptor implements IEntityAdaptor
}
}
- public void addProperty(IEntityPropertyAdaptor property)
+ private void initPropertiesMap(Collection<IEntityPropertyAdaptor> properties)
+ {
+ propertiesByCode = new HashMap<String, IEntityPropertyAdaptor>();
+
+ if (properties != null)
+ {
+ for (IEntityPropertyAdaptor property : properties)
+ {
+ addProperty(property);
+ }
+ }
+ }
+
+ private Map<String, IEntityPropertyAdaptor> propertiesMap()
+ {
+ if (propertiesByCode == null)
+ {
+ initPropertiesMap();
+ }
+
+ return propertiesByCode;
+ }
+
+ private void addProperty(IEntityPropertyAdaptor property)
{
- propertiesByCode.put(property.propertyTypeCode().toUpperCase(), property);
+ propertiesMap().put(property.propertyTypeCode().toUpperCase(), property);
}
@Override
@@ -129,7 +166,7 @@ public class AbstractEntityAdaptor implements IEntityAdaptor
@Override
public IEntityPropertyAdaptor property(String propertyTypeCode)
{
- return propertiesByCode.get(propertyTypeCode.toUpperCase());
+ return propertiesMap().get(propertyTypeCode.toUpperCase());
}
@Override
@@ -149,7 +186,7 @@ public class AbstractEntityAdaptor implements IEntityAdaptor
@Override
public Collection<IEntityPropertyAdaptor> properties()
{
- return propertiesByCode.values();
+ return propertiesMap().values();
}
protected Query regexpConstraint(String field, String value)
@@ -159,8 +196,8 @@ public class AbstractEntityAdaptor implements IEntityAdaptor
protected Query constraint(String field, String value)
{
- return LuceneQueryBuilder.parseQuery(field, value, LuceneQueryBuilder
- .createSearchAnalyzer());
+ return LuceneQueryBuilder.parseQuery(field, value,
+ LuceneQueryBuilder.createSearchAnalyzer());
}
protected Query and(Query... queries)
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/ExperimentAdaptor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/ExperimentAdaptor.java
index 7da2d285250..72e245d1c18 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/ExperimentAdaptor.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/ExperimentAdaptor.java
@@ -45,9 +45,8 @@ public class ExperimentAdaptor extends AbstractEntityAdaptor implements IExperim
public ExperimentAdaptor(ExperimentPE experimentPE, IDynamicPropertyEvaluator evaluator,
Session session)
{
- super(experimentPE.getCode(), evaluator);
+ super(experimentPE, evaluator);
this.session = session;
- initProperties(experimentPE);
this.experimentPE = experimentPE;
}
@@ -71,8 +70,7 @@ public class ExperimentAdaptor extends AbstractEntityAdaptor implements IExperim
@Override
public Iterable<ISampleAdaptor> samplesOfType(String typeRegexp)
{
- Query typeConstraint =
- regexpConstraint(ENTITY_TYPE_CODE_FIELD, typeRegexp.toLowerCase());
+ Query typeConstraint = regexpConstraint(ENTITY_TYPE_CODE_FIELD, typeRegexp.toLowerCase());
Query experimentCodeConstraint =
constraint(SearchFieldConstants.EXPERIMENT_ID, Long.toString(experimentPE.getId()));
Query query = and(typeConstraint, experimentCodeConstraint);
@@ -90,8 +88,7 @@ public class ExperimentAdaptor extends AbstractEntityAdaptor implements IExperim
@Override
public Iterable<IDataAdaptor> dataSetsOfType(String typeRegexp)
{
- Query typeConstraint =
- regexpConstraint(ENTITY_TYPE_CODE_FIELD, typeRegexp.toLowerCase());
+ Query typeConstraint = regexpConstraint(ENTITY_TYPE_CODE_FIELD, typeRegexp.toLowerCase());
Query experimentCodeConstraint =
constraint(SearchFieldConstants.EXPERIMENT_ID, Long.toString(experimentPE.getId()));
Query query = and(typeConstraint, experimentCodeConstraint);
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/ExternalDataAdaptor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/ExternalDataAdaptor.java
index c63475b6db0..d7fecae3714 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/ExternalDataAdaptor.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/ExternalDataAdaptor.java
@@ -44,9 +44,8 @@ public class ExternalDataAdaptor extends AbstractEntityAdaptor implements IDataA
public ExternalDataAdaptor(DataPE externalDataPE, IDynamicPropertyEvaluator evaluator,
Session session)
{
- super(externalDataPE.getCode(), evaluator);
+ super(externalDataPE, evaluator);
this.session = session;
- initProperties(externalDataPE);
this.externalDataPE = externalDataPE;
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/MaterialAdaptor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/MaterialAdaptor.java
index 327c50be52b..b76232493b4 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/MaterialAdaptor.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/MaterialAdaptor.java
@@ -33,8 +33,7 @@ public class MaterialAdaptor extends AbstractEntityAdaptor implements IMaterialA
public MaterialAdaptor(MaterialPE MaterialPE, IDynamicPropertyEvaluator evaluator)
{
- super(MaterialPE.getCode(), evaluator);
- initProperties(MaterialPE);
+ super(MaterialPE, evaluator);
this.MaterialPE = MaterialPE;
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/SampleAdaptor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/SampleAdaptor.java
index 2487d63405a..37e2bb720b7 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/SampleAdaptor.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/SampleAdaptor.java
@@ -42,9 +42,8 @@ public class SampleAdaptor extends AbstractEntityAdaptor implements ISampleAdapt
public SampleAdaptor(SamplePE samplePE, IDynamicPropertyEvaluator evaluator, Session session)
{
- super(samplePE.getCode(), evaluator);
+ super(samplePE, evaluator);
this.session = session;
- initProperties(samplePE);
this.samplePE = samplePE;
}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/DataPE.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/DataPE.java
index d2c6033dc34..fde907ad019 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/DataPE.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/DataPE.java
@@ -42,6 +42,7 @@ import javax.persistence.Version;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Pattern;
+import org.hibernate.annotations.BatchSize;
import org.hibernate.annotations.Cache;
import org.hibernate.annotations.CacheConcurrencyStrategy;
import org.hibernate.annotations.Fetch;
@@ -674,7 +675,7 @@ public class DataPE extends AbstractIdAndCodeHolder<DataPE> implements
@OneToMany(fetch = FetchType.LAZY, cascade = CascadeType.ALL, mappedBy = "entity", orphanRemoval = true)
@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
@IndexedEmbedded(prefix = SearchFieldConstants.PREFIX_PROPERTIES)
- @Fetch(FetchMode.SUBSELECT)
+ @BatchSize(size = 100)
private Set<DataSetPropertyPE> getDataSetProperties()
{
return properties;
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/ExperimentPE.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/ExperimentPE.java
index 8db2823f70d..837341cc6ac 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/ExperimentPE.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/ExperimentPE.java
@@ -44,6 +44,7 @@ import javax.validation.constraints.Pattern;
import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;
+import org.hibernate.annotations.BatchSize;
import org.hibernate.annotations.Cache;
import org.hibernate.annotations.CacheConcurrencyStrategy;
import org.hibernate.annotations.Fetch;
@@ -281,7 +282,7 @@ public class ExperimentPE extends AttachmentHolderPE implements
@OneToMany(fetch = FetchType.LAZY, cascade = CascadeType.ALL, mappedBy = "entity", orphanRemoval = true)
@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
@IndexedEmbedded(prefix = SearchFieldConstants.PREFIX_PROPERTIES)
- @Fetch(FetchMode.SUBSELECT)
+ @BatchSize(size = 100)
private Set<ExperimentPropertyPE> getExperimentProperties()
{
return properties;
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/MaterialPE.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/MaterialPE.java
index 53d831e4b4e..6bb16b42a72 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/MaterialPE.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/MaterialPE.java
@@ -41,6 +41,7 @@ import javax.validation.constraints.NotNull;
import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;
+import org.hibernate.annotations.BatchSize;
import org.hibernate.annotations.Cache;
import org.hibernate.annotations.CacheConcurrencyStrategy;
import org.hibernate.annotations.Fetch;
@@ -76,7 +77,8 @@ import ch.systemsx.cisd.openbis.generic.shared.util.HibernateUtils;
ColumnNames.DATABASE_INSTANCE_COLUMN }))
@Indexed(index = "MaterialPE")
public class MaterialPE implements IIdAndCodeHolder, Comparable<MaterialPE>,
- IEntityInformationWithPropertiesHolder, Serializable, IMatchingEntity, IEntityWithMetaprojects
+ IEntityInformationWithPropertiesHolder, Serializable, IMatchingEntity,
+ IEntityWithMetaprojects
{
private static final long serialVersionUID = IServer.VERSION;
@@ -215,7 +217,7 @@ public class MaterialPE implements IIdAndCodeHolder, Comparable<MaterialPE>,
@OneToMany(fetch = FetchType.LAZY, cascade = CascadeType.ALL, mappedBy = "entity", orphanRemoval = true)
@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
@IndexedEmbedded(prefix = SearchFieldConstants.PREFIX_PROPERTIES)
- @Fetch(FetchMode.SUBSELECT)
+ @BatchSize(size = 100)
private Set<MaterialPropertyPE> getMaterialProperties()
{
return properties;
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/SamplePE.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/SamplePE.java
index 04474876de3..41e62b1b7ec 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/SamplePE.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/dto/SamplePE.java
@@ -46,6 +46,7 @@ import javax.validation.constraints.Pattern;
import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;
+import org.hibernate.annotations.BatchSize;
import org.hibernate.annotations.Cache;
import org.hibernate.annotations.CacheConcurrencyStrategy;
import org.hibernate.annotations.Check;
@@ -373,7 +374,7 @@ public class SamplePE extends AttachmentHolderPE implements IIdAndCodeHolder, Co
@OneToMany(fetch = FetchType.LAZY, cascade = CascadeType.ALL, mappedBy = "entity", orphanRemoval = true)
@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
@IndexedEmbedded(prefix = SearchFieldConstants.PREFIX_PROPERTIES)
- @Fetch(FetchMode.SUBSELECT)
+ @BatchSize(size = 100)
private Set<SamplePropertyPE> getSampleProperties()
{
return properties;
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/DynamicPropertyEvaluatorTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/DynamicPropertyEvaluatorTest.java
index 3be83611714..be713d1acde 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/DynamicPropertyEvaluatorTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/DynamicPropertyEvaluatorTest.java
@@ -522,15 +522,7 @@ public class DynamicPropertyEvaluatorTest extends AbstractBOTest
private static IEntityAdaptor createEntity(final String code,
final Collection<IEntityPropertyAdaptor> properties)
{
- final AbstractEntityAdaptor result = new AbstractEntityAdaptor(code);
- if (properties != null)
- {
- for (IEntityPropertyAdaptor property : properties)
- {
- result.addProperty(property);
- }
- }
- return result;
+ return new AbstractEntityAdaptor(code, properties);
}
private static IEntityPropertyAdaptor createProperty(final String propertyTypeCode,
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/DynamicPropertyCalculatorTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/DynamicPropertyCalculatorTest.java
index e0ac229ab85..99b0cc0bb84 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/DynamicPropertyCalculatorTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/dynamic_property/calculator/DynamicPropertyCalculatorTest.java
@@ -23,10 +23,6 @@ import org.testng.AssertJUnit;
import org.testng.annotations.Test;
import ch.systemsx.cisd.common.jython.evaluator.EvaluatorException;
-import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator.AbstractEntityAdaptor;
-import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator.BasicPropertyAdaptor;
-import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator.DynamicPropertyCalculator;
-import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator.XmlPropertyAdaptor;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator.api.IEntityAdaptor;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.calculator.api.IEntityPropertyAdaptor;
import ch.systemsx.cisd.openbis.generic.shared.basic.dto.MaterialIdentifier;
@@ -152,15 +148,7 @@ public class DynamicPropertyCalculatorTest extends AssertJUnit
private static IEntityAdaptor createEntity(final String code,
final Collection<IEntityPropertyAdaptor> properties)
{
- final AbstractEntityAdaptor result = new AbstractEntityAdaptor(code);
- if (properties != null)
- {
- for (IEntityPropertyAdaptor property : properties)
- {
- result.addProperty(property);
- }
- }
- return result;
+ return new AbstractEntityAdaptor(code, properties);
}
private static IEntityPropertyAdaptor createProperty(final String propertyTypeCode,
--
GitLab