From 0ff907665ee2f4cdee0c5632dfe7e72e54ed7243 Mon Sep 17 00:00:00 2001 From: pkupczyk <pkupczyk> Date: Fri, 18 Sep 2015 14:04:34 +0000 Subject: [PATCH] SSDM-2498 : V3 AS API - analyze performance of searchSamples after rewrite to SQL - some performance improvements: - AbstractCachingTranslator.doShouldTranslate removeAll problem when shouldTranslate returned a list - improved structure of TranslationCache - reduced number of reflection calls in SortAndPage SVN: 34672 --- .../translator/AbstractCachingTranslator.java | 69 +++--- .../api/v3/translator/TranslationCache.java | 117 ++++++----- .../sql/IObjectAuthorizationValidator.java | 3 +- .../ExperimentAuthorizationSqlValidator.java | 11 +- .../sql/ExperimentSqlTranslator.java | 2 +- .../sql/ProjectAuthorizationSqlValidator.java | 7 +- .../project/sql/ProjectSqlTranslator.java | 3 +- .../sql/SampleAuthorizationSqlValidator.java | 11 +- .../sample/sql/SampleSqlTranslator.java | 2 +- .../sql/SpaceAuthorizationSqlValidator.java | 7 +- .../entity/space/sql/SpaceSqlTranslator.java | 3 +- .../entity/tag/sql/TagSqlTranslator.java | 7 +- .../v3/dto/fetchoptions/sort/SortAndPage.java | 196 ++++++++++++++---- 13 files changed, 295 insertions(+), 143 deletions(-) diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/AbstractCachingTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/AbstractCachingTranslator.java index 3124b17ba4a..28616c2238d 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/AbstractCachingTranslator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/AbstractCachingTranslator.java @@ -21,9 +21,11 @@ import java.util.Collections; import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; +import java.util.Set; import org.apache.log4j.Logger; +import ch.ethz.sis.openbis.generic.server.api.v3.translator.TranslationCache.CacheEntry; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.FetchOptions; import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; @@ -64,7 +66,9 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> for (I input : inputs) { - if (cache.hasTranslatedObject(namespace, getId(input))) + CacheEntry cacheEntry = cache.getEntry(getObjectCacheKey(input)); + + if (cacheEntry.isTranslatedObjectSet()) { handleAlreadyTranslatedInput(context, input, translated, updated, fetchOptions); } else @@ -89,35 +93,34 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> @SuppressWarnings("unchecked") private final void handleAlreadyTranslatedInput(TranslationContext context, I input, Map<I, O> translated, Map<I, O> updated, F fetchOptions) { - Long id = getId(input); TranslationCache cache = context.getTranslationCache(); - - O output = (O) cache.getTranslatedObject(namespace, id); + CacheEntry cacheEntry = cache.getEntry(getObjectCacheKey(input)); + O output = (O) cacheEntry.getTranslatedObject(); if (output == null) { if (operationLog.isDebugEnabled()) { - operationLog.debug("Found that object was already rejected from translation: " + id); + operationLog.debug("Found that object was already rejected from translation: " + getObjectId(input)); } } else { if (operationLog.isDebugEnabled()) { - operationLog.debug("Found in cache: " + output.getClass() + " with id: " + id); + operationLog.debug("Found in cache: " + output.getClass() + " with id: " + getObjectId(input)); } - if (cache.isFetchedWithOptions(output, fetchOptions)) + if (cacheEntry.isTranslatedWithFetchOptions(fetchOptions)) { translated.put(input, output); } else { if (operationLog.isDebugEnabled()) { - operationLog.debug("Updating from cache: " + output.getClass() + " with id: " + id); + operationLog.debug("Updating from cache: " + output.getClass() + " with id: " + getObjectId(input)); } - cache.setFetchedWithOptions(output, fetchOptions); + cacheEntry.addTranslatedWithFetchOptions(fetchOptions); updated.put(input, output); translated.put(input, output); } @@ -126,27 +129,28 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> private void handleNewInput(TranslationContext context, I input, Map<I, O> translated, Map<I, O> updated, F fetchOptions) { - Long id = getId(input); O output = createObject(context, input, fetchOptions); - TranslationCache cache = context.getTranslationCache(); if (operationLog.isDebugEnabled()) { - operationLog.debug("Created: " + output.getClass() + " with id: " + id); + operationLog.debug("Created: " + output.getClass() + " with id: " + getObjectId(input)); } - cache.putTranslatedObject(namespace, id, output); - cache.setFetchedWithOptions(output, fetchOptions); + TranslationCache cache = context.getTranslationCache(); + CacheEntry cacheEntry = cache.getEntry(getObjectCacheKey(input)); + cacheEntry.setTranslatedObject(output); + cacheEntry.addTranslatedWithFetchOptions(fetchOptions); + updated.put(input, output); translated.put(input, output); if (operationLog.isDebugEnabled()) { - operationLog.debug("Updating created: " + output.getClass() + " with id: " + id); + operationLog.debug("Updating created: " + output.getClass() + " with id: " + getObjectId(input)); } } - private Long getId(I input) + private Long getObjectId(I input) { if (input instanceof IIdHolder) { @@ -160,6 +164,11 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> } } + private String getObjectCacheKey(I input) + { + return namespace + getObjectId(input); + } + private final Collection<I> doShouldTranslate(TranslationContext context, Collection<I> inputs, F fetchOptions) { TranslationCache cache = context.getTranslationCache(); @@ -168,11 +177,12 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> for (I input : inputs) { - Long id = getId(input); + CacheEntry cacheEntry = cache.getEntry(getObjectCacheKey(input)); - if (cache.hasShouldTranslateObject(namespace, id)) + if (cacheEntry.isShouldTranslateSet()) { - boolean should = cache.getShouldTranslateObject(namespace, id); + boolean should = cacheEntry.getShouldTranslate(); + if (should) { toTranslate.add(input); @@ -180,6 +190,8 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> if (operationLog.isDebugEnabled()) { + Long id = getObjectId(input); + if (should) { operationLog.debug("Found in cache that object with id: " + id + " should be translated"); @@ -199,22 +211,25 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> for (I input : checked) { - Long id = getId(input); - cache.putShouldTranslateObject(namespace, id, true); + CacheEntry cacheEntry = cache.getEntry(getObjectCacheKey(input)); + cacheEntry.setShouldTranslate(true); + if (operationLog.isDebugEnabled()) { - operationLog.debug("Should translate object with id: " + id); + operationLog.debug("Should translate object with id: " + getObjectId(input)); } } toCheck.removeAll(checked); + for (I input : toCheck) { - Long id = getId(input); - cache.putShouldTranslateObject(namespace, id, false); + CacheEntry cacheEntry = cache.getEntry(getObjectCacheKey(input)); + cacheEntry.setShouldTranslate(false); + if (operationLog.isDebugEnabled()) { - operationLog.debug("Should NOT translate object with id: " + id); + operationLog.debug("Should NOT translate object with id: " + getObjectId(input)); } } @@ -225,9 +240,9 @@ public abstract class AbstractCachingTranslator<I, O, F extends FetchOptions<?>> * Override this method if you want to conditionally skip translation (e.g. when the input object is not visible for a user the translation is * performed for) */ - protected Collection<I> shouldTranslate(TranslationContext context, Collection<I> inputs, F fetchOptions) + protected Set<I> shouldTranslate(TranslationContext context, Collection<I> inputs, F fetchOptions) { - Collection<I> result = new LinkedHashSet<I>(); + Set<I> result = new LinkedHashSet<I>(); for (I input : inputs) { diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/TranslationCache.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/TranslationCache.java index 2a2f946f733..b5b8d39b912 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/TranslationCache.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/TranslationCache.java @@ -2,87 +2,96 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator; import java.util.HashMap; import java.util.HashSet; -import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; public class TranslationCache { - /** - * Map storing information whether an object with the given namespace and object id (key) should be translated (value) - */ - private Map<String, Boolean> shouldTranslateObjects = new HashMap<String, Boolean>(); + private Map<String, CacheEntry> entries = new HashMap<String, CacheEntry>(); - /** - * Map storing information if the given fetch options (value) has already been used for the given object (key) - */ - private Map<Object, Set<Object>> usedFetchOptions = new IdentityHashMap<Object, Set<Object>>(); + public CacheEntry getEntry(String identifier) + { + CacheEntry cacheEntry = entries.get(identifier); - /** - * Map storing already translated object (value) for the given namespace and object id (key) - */ - private Map<String, Object> translatedObjects = new HashMap<String, Object>(); + if (cacheEntry == null) + { + cacheEntry = new CacheEntry(); + entries.put(identifier, cacheEntry); + } - public boolean hasShouldTranslateObject(String namespace, Long objectId) - { - return shouldTranslateObjects.containsKey(getObjectKey(namespace, objectId)); + return cacheEntry; } - public boolean getShouldTranslateObject(String namespace, Long objectId) + public static class CacheEntry { - return shouldTranslateObjects.get(getObjectKey(namespace, objectId)); - } - public void putShouldTranslateObject(String namespace, Long objectId, boolean shouldTranslate) - { - shouldTranslateObjects.put(getObjectKey(namespace, objectId), shouldTranslate); - } + private boolean shouldTranslateSet; - public boolean hasTranslatedObject(String namespace, Long objectId) - { - return translatedObjects.containsKey(getObjectKey(namespace, objectId)); - } + private Boolean shouldTranslate; - public Object getTranslatedObject(String namespace, Long objectId) - { - return translatedObjects.get(getObjectKey(namespace, objectId)); - } + private boolean translatedObjectSet; - public void putTranslatedObject(String namespace, Long objectId, Object object) - { - translatedObjects.put(getObjectKey(namespace, objectId), object); - } + private Object translatedObject; - public boolean isFetchedWithOptions(Object object, Object fetchOptions) - { - Set<Object> set = usedFetchOptions.get(object); + private Set<Object> usedFetchOptions; - if (set == null) + private CacheEntry() { - return false; - } else + } + + public boolean isShouldTranslateSet() { - return set.contains(fetchOptions); + return shouldTranslateSet; } - } - public void setFetchedWithOptions(Object object, Object fetchOptions) - { - Set<Object> set = usedFetchOptions.get(object); + public Boolean getShouldTranslate() + { + return shouldTranslate; + } - if (set == null) + public void setShouldTranslate(boolean shouldTranslate) { - set = new HashSet<Object>(); - usedFetchOptions.put(object, set); + this.shouldTranslate = shouldTranslate; + this.shouldTranslateSet = true; } - set.add(fetchOptions); - } + public boolean isTranslatedObjectSet() + { + return translatedObjectSet; + } + + public Object getTranslatedObject() + { + return translatedObject; + } + + public void setTranslatedObject(Object translatedObject) + { + this.translatedObject = translatedObject; + this.translatedObjectSet = true; + } + + public boolean isTranslatedWithFetchOptions(Object fetchOptions) + { + if (usedFetchOptions == null) + { + return false; + } else + { + return usedFetchOptions.contains(fetchOptions); + } + } + + public void addTranslatedWithFetchOptions(Object fetchOptions) + { + if (usedFetchOptions == null) + { + usedFetchOptions = new HashSet<Object>(); + } + usedFetchOptions.add(fetchOptions); + } - private String getObjectKey(String namespace, Long objectId) - { - return namespace + "." + objectId; } } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/common/sql/IObjectAuthorizationValidator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/common/sql/IObjectAuthorizationValidator.java index 8658465afbf..8ff772e3be5 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/common/sql/IObjectAuthorizationValidator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/common/sql/IObjectAuthorizationValidator.java @@ -17,6 +17,7 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator.entity.common.sql; import java.util.Collection; +import java.util.Set; import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE; @@ -26,6 +27,6 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE; public interface IObjectAuthorizationValidator { - public Collection<Long> validate(PersonPE person, Collection<Long> objectIds); + public Set<Long> validate(PersonPE person, Collection<Long> objectIds); } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/experiment/sql/ExperimentAuthorizationSqlValidator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/experiment/sql/ExperimentAuthorizationSqlValidator.java index 4aa5d45dee9..8940a5651a5 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/experiment/sql/ExperimentAuthorizationSqlValidator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/experiment/sql/ExperimentAuthorizationSqlValidator.java @@ -19,13 +19,14 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator.entity.experiment.s import it.unimi.dsi.fastutil.longs.LongOpenHashSet; import java.util.Collection; -import java.util.LinkedList; +import java.util.HashSet; import java.util.List; - -import org.springframework.stereotype.Component; +import java.util.Set; import net.lemnik.eodsql.QueryTool; +import org.springframework.stereotype.Component; + import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.experiment.ExperimentIdentifier; import ch.systemsx.cisd.openbis.generic.server.authorization.validator.ExperimentByIdentiferValidator; import ch.systemsx.cisd.openbis.generic.shared.basic.IIdentifierHolder; @@ -39,12 +40,12 @@ public class ExperimentAuthorizationSqlValidator implements IExperimentAuthoriza { @Override - public Collection<Long> validate(PersonPE person, Collection<Long> experimentIds) + public Set<Long> validate(PersonPE person, Collection<Long> experimentIds) { ExperimentQuery query = QueryTool.getManagedQuery(ExperimentQuery.class); List<ExperimentAuthorizationRecord> records = query.getAuthorizations(new LongOpenHashSet(experimentIds)); ExperimentByIdentiferValidator validator = new ExperimentByIdentiferValidator(); - List<Long> result = new LinkedList<Long>(); + Set<Long> result = new HashSet<Long>(); for (ExperimentAuthorizationRecord record : records) { diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/experiment/sql/ExperimentSqlTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/experiment/sql/ExperimentSqlTranslator.java index aaef19f2f3d..99e4e49d139 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/experiment/sql/ExperimentSqlTranslator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/experiment/sql/ExperimentSqlTranslator.java @@ -82,7 +82,7 @@ public class ExperimentSqlTranslator extends AbstractCachingTranslator<Long, Exp private IExperimentHistorySqlTranslator historyTranslator; @Override - protected Collection<Long> shouldTranslate(TranslationContext context, Collection<Long> experimentIds, ExperimentFetchOptions fetchOptions) + protected Set<Long> shouldTranslate(TranslationContext context, Collection<Long> experimentIds, ExperimentFetchOptions fetchOptions) { return authorizationValidator.validate(context.getSession().tryGetPerson(), experimentIds); } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/project/sql/ProjectAuthorizationSqlValidator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/project/sql/ProjectAuthorizationSqlValidator.java index bc8fbd43160..7249141be6e 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/project/sql/ProjectAuthorizationSqlValidator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/project/sql/ProjectAuthorizationSqlValidator.java @@ -19,8 +19,9 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator.entity.project.sql; import it.unimi.dsi.fastutil.longs.LongOpenHashSet; import java.util.Collection; -import java.util.LinkedList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import net.lemnik.eodsql.QueryTool; @@ -39,12 +40,12 @@ public class ProjectAuthorizationSqlValidator implements IProjectAuthorizationSq { @Override - public Collection<Long> validate(PersonPE person, Collection<Long> projectIds) + public Set<Long> validate(PersonPE person, Collection<Long> projectIds) { ProjectQuery query = QueryTool.getManagedQuery(ProjectQuery.class); List<ProjectAuthorizationRecord> records = query.getAuthorizations(new LongOpenHashSet(projectIds)); ProjectByIdentiferValidator validator = new ProjectByIdentiferValidator(); - List<Long> result = new LinkedList<Long>(); + Set<Long> result = new HashSet<Long>(); for (ProjectAuthorizationRecord record : records) { diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/project/sql/ProjectSqlTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/project/sql/ProjectSqlTranslator.java index 58c655e7c19..0fce4cd73bb 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/project/sql/ProjectSqlTranslator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/project/sql/ProjectSqlTranslator.java @@ -18,6 +18,7 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator.entity.project.sql; import java.util.Collection; import java.util.List; +import java.util.Set; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -67,7 +68,7 @@ public class ProjectSqlTranslator extends AbstractCachingTranslator<Long, Projec private IProjectHistorySqlTranslator historyTranslator; @Override - protected Collection<Long> shouldTranslate(TranslationContext context, Collection<Long> projectIds, ProjectFetchOptions fetchOptions) + protected Set<Long> shouldTranslate(TranslationContext context, Collection<Long> projectIds, ProjectFetchOptions fetchOptions) { return authorizationValidator.validate(context.getSession().tryGetPerson(), projectIds); } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/sample/sql/SampleAuthorizationSqlValidator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/sample/sql/SampleAuthorizationSqlValidator.java index 008e8649c31..0db2f2e4210 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/sample/sql/SampleAuthorizationSqlValidator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/sample/sql/SampleAuthorizationSqlValidator.java @@ -19,13 +19,14 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator.entity.sample.sql; import it.unimi.dsi.fastutil.longs.LongOpenHashSet; import java.util.Collection; -import java.util.LinkedList; +import java.util.HashSet; import java.util.List; - -import org.springframework.stereotype.Component; +import java.util.Set; import net.lemnik.eodsql.QueryTool; +import org.springframework.stereotype.Component; + import ch.ethz.sis.openbis.generic.shared.api.v3.dto.id.sample.SampleIdentifier; import ch.systemsx.cisd.openbis.generic.server.authorization.validator.SampleByIdentiferValidator; import ch.systemsx.cisd.openbis.generic.shared.basic.IIdentifierHolder; @@ -39,12 +40,12 @@ public class SampleAuthorizationSqlValidator implements ISampleAuthorizationSqlV { @Override - public Collection<Long> validate(PersonPE person, Collection<Long> sampleIds) + public Set<Long> validate(PersonPE person, Collection<Long> sampleIds) { SampleQuery query = QueryTool.getManagedQuery(SampleQuery.class); List<SampleAuthorizationRecord> records = query.getAuthorizations(new LongOpenHashSet(sampleIds)); SampleByIdentiferValidator validator = new SampleByIdentiferValidator(); - List<Long> result = new LinkedList<Long>(); + Set<Long> result = new HashSet<Long>(); for (SampleAuthorizationRecord record : records) { diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/sample/sql/SampleSqlTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/sample/sql/SampleSqlTranslator.java index 9ab417a6194..224fcb7cbdc 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/sample/sql/SampleSqlTranslator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/sample/sql/SampleSqlTranslator.java @@ -93,7 +93,7 @@ public class SampleSqlTranslator extends AbstractCachingTranslator<Long, Sample, private ISampleModifierSqlTranslator modifierTranslator; @Override - protected Collection<Long> shouldTranslate(TranslationContext context, Collection<Long> sampleIds, SampleFetchOptions fetchOptions) + protected Set<Long> shouldTranslate(TranslationContext context, Collection<Long> sampleIds, SampleFetchOptions fetchOptions) { return authorizationValidator.validate(context.getSession().tryGetPerson(), sampleIds); } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/space/sql/SpaceAuthorizationSqlValidator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/space/sql/SpaceAuthorizationSqlValidator.java index 8c116a74ed4..1e606d8de11 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/space/sql/SpaceAuthorizationSqlValidator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/space/sql/SpaceAuthorizationSqlValidator.java @@ -19,8 +19,9 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator.entity.space.sql; import it.unimi.dsi.fastutil.longs.LongOpenHashSet; import java.util.Collection; -import java.util.LinkedList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import net.lemnik.eodsql.QueryTool; @@ -38,12 +39,12 @@ public class SpaceAuthorizationSqlValidator implements ISpaceAuthorizationSqlVal { @Override - public Collection<Long> validate(PersonPE person, Collection<Long> spaceIds) + public Set<Long> validate(PersonPE person, Collection<Long> spaceIds) { SpaceQuery query = QueryTool.getManagedQuery(SpaceQuery.class); List<SpaceAuthorizationRecord> records = query.getAuthorizations(new LongOpenHashSet(spaceIds)); SimpleSpaceValidator validator = new SimpleSpaceValidator(); - List<Long> result = new LinkedList<Long>(); + Set<Long> result = new HashSet<Long>(); for (SpaceAuthorizationRecord record : records) { diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/space/sql/SpaceSqlTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/space/sql/SpaceSqlTranslator.java index a3c1c410e8b..0938437c23e 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/space/sql/SpaceSqlTranslator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/space/sql/SpaceSqlTranslator.java @@ -18,6 +18,7 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator.entity.space.sql; import java.util.Collection; import java.util.List; +import java.util.Set; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -54,7 +55,7 @@ public class SpaceSqlTranslator extends AbstractCachingTranslator<Long, Space, S private ISpaceBaseSqlTranslator baseTranslator; @Override - protected Collection<Long> shouldTranslate(TranslationContext context, Collection<Long> spaceIds, SpaceFetchOptions fetchOptions) + protected Set<Long> shouldTranslate(TranslationContext context, Collection<Long> spaceIds, SpaceFetchOptions fetchOptions) { return authorizationValidator.validate(context.getSession().tryGetPerson(), spaceIds); } diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/tag/sql/TagSqlTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/tag/sql/TagSqlTranslator.java index 3ffaa02e4c0..07ee98620db 100644 --- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/tag/sql/TagSqlTranslator.java +++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/api/v3/translator/entity/tag/sql/TagSqlTranslator.java @@ -19,8 +19,9 @@ package ch.ethz.sis.openbis.generic.server.api.v3.translator.entity.tag.sql; import it.unimi.dsi.fastutil.longs.LongOpenHashSet; import java.util.Collection; -import java.util.LinkedList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import net.lemnik.eodsql.QueryTool; @@ -48,11 +49,11 @@ public class TagSqlTranslator extends AbstractCachingTranslator<Long, Tag, TagFe private ITagOwnerSqlTranslator ownerTranslator; @Override - protected Collection<Long> shouldTranslate(TranslationContext context, Collection<Long> tagIds, TagFetchOptions fetchOptions) + protected Set<Long> shouldTranslate(TranslationContext context, Collection<Long> tagIds, TagFetchOptions fetchOptions) { TagQuery query = QueryTool.getManagedQuery(TagQuery.class); List<TagAuthorizationRecord> records = query.getAuthorizations(new LongOpenHashSet(tagIds)); - Collection<Long> result = new LinkedList<Long>(); + Set<Long> result = new HashSet<Long>(); for (TagAuthorizationRecord record : records) { diff --git a/openbis_api/source/java/ch/ethz/sis/openbis/generic/shared/api/v3/dto/fetchoptions/sort/SortAndPage.java b/openbis_api/source/java/ch/ethz/sis/openbis/generic/shared/api/v3/dto/fetchoptions/sort/SortAndPage.java index 83b3c0c41f3..7125b0472e4 100644 --- a/openbis_api/source/java/ch/ethz/sis/openbis/generic/shared/api/v3/dto/fetchoptions/sort/SortAndPage.java +++ b/openbis_api/source/java/ch/ethz/sis/openbis/generic/shared/api/v3/dto/fetchoptions/sort/SortAndPage.java @@ -16,17 +16,21 @@ package ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.sort; +import java.beans.PropertyDescriptor; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; +import org.springframework.beans.BeanUtils; + import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.FetchOptions; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.sort.view.AbstractCollectionView; import ch.ethz.sis.openbis.generic.shared.api.v3.dto.fetchoptions.sort.view.ListView; @@ -41,6 +45,8 @@ public class SortAndPage private Set processed = new HashSet(); + private MethodsCache methodsCache = new MethodsCache(); + public <T, C extends Collection<T>> C sortAndPage(C objects, FetchOptions fo) { C newObjects = objects; @@ -135,58 +141,58 @@ public class SortAndPage return; } - try + FetchOptionsMethods foMethods = methodsCache.getFetchOptionsMethods(fo); + + for (Object object : objects) { - for (Object object : objects) + if (processed.contains(object)) { - if (processed.contains(object)) - { - continue; - } else - { - processed.add(object); - } + continue; + } else + { + processed.add(object); + } - // TODO find the methods only once for given class instead of doing it for each object - for (Method method : fo.getClass().getMethods()) + ObjectMethods objectMethods = methodsCache.getObjectMethods(object); + + for (String fieldName : foMethods.getFieldNames()) + { + try { - if (method.getName().startsWith("has") && false == method.getName().equals("hashCode")) - { - String field = method.getName().substring(3); - boolean has = (Boolean) method.invoke(fo); + Method hasMethod = foMethods.getHasMethod(fieldName); + boolean has = (Boolean) hasMethod.invoke(fo); - if (has) - { - Method withMethod = fo.getClass().getMethod("with" + field); - FetchOptions subFo = (FetchOptions) withMethod.invoke(fo); + if (has) + { + Method withMethod = foMethods.getWithMethod(fieldName); + FetchOptions subFo = (FetchOptions) withMethod.invoke(fo); - Method getMethod = object.getClass().getMethod("get" + field); - Method setMethod = object.getClass().getMethod("set" + field, getMethod.getReturnType()); + Method getMethod = objectMethods.getGetMethod(fieldName); + Method setMethod = objectMethods.getSetMethod(fieldName); - Object value = getMethod.invoke(object); + Object value = getMethod.invoke(object); - if (value != null) + if (value != null) + { + if (value instanceof Collection) { - if (value instanceof Collection) - { - Collection newValue = sortAndPage((Collection) value, subFo); - setMethod.invoke(object, newValue); - } else if (value instanceof Map) - { - sortAndPage(((Map) value).values(), subFo); - } else - { - Collection newValue = sortAndPage(Collections.singleton(value), subFo); - setMethod.invoke(object, newValue.iterator().next()); - } + Collection newValue = sortAndPage((Collection) value, subFo); + setMethod.invoke(object, newValue); + } else if (value instanceof Map) + { + sortAndPage(((Map) value).values(), subFo); + } else + { + Collection newValue = sortAndPage(Collections.singleton(value), subFo); + setMethod.invoke(object, newValue.iterator().next()); } } } + } catch (Exception e) + { + throw new RuntimeException("Sorting and paging failed for object: + " + object + " and fieldName: " + fieldName, e); } } - } catch (Exception e) - { - throw new RuntimeException(e); } } @@ -245,4 +251,118 @@ public class SortAndPage return null; } + private class MethodsCache + { + + private Map<Class, Object> cache = new HashMap<Class, Object>(); + + public FetchOptionsMethods getFetchOptionsMethods(Object fo) + { + FetchOptionsMethods methods = (FetchOptionsMethods) cache.get(fo.getClass()); + + if (methods == null) + { + methods = new FetchOptionsMethods(fo.getClass()); + cache.put(fo.getClass(), methods); + } + + return methods; + } + + public ObjectMethods getObjectMethods(Object object) + { + ObjectMethods methods = (ObjectMethods) cache.get(object.getClass()); + + if (methods == null) + { + methods = new ObjectMethods(object.getClass()); + cache.put(object.getClass(), methods); + } + + return methods; + } + + } + + private class FetchOptionsMethods + { + + private Collection<String> fieldNames = new HashSet<String>(); + + private Map<String, Method> hasMethods = new HashMap<String, Method>(); + + private Map<String, Method> withMethods = new HashMap<String, Method>(); + + public FetchOptionsMethods(Class clazz) + { + try + { + for (Method method : clazz.getMethods()) + { + if (method.getName().startsWith("has") && false == method.getName().equals("hashCode")) + { + String fieldName = method.getName().substring(3); + + hasMethods.put(fieldName, method); + + Method withMethod = clazz.getMethod("with" + fieldName); + withMethods.put(fieldName, withMethod); + + fieldNames.add(fieldName); + } + } + } catch (Exception e) + { + throw new RuntimeException("Finding methods for fetch options class: " + clazz.getName() + " failed.", e); + } + } + + public Collection<String> getFieldNames() + { + return fieldNames; + } + + public Method getHasMethod(String fieldName) + { + return hasMethods.get(fieldName); + } + + public Method getWithMethod(String fieldName) + { + return withMethods.get(fieldName); + } + + } + + private class ObjectMethods + { + + private Map<String, Method> getMethods = new HashMap<String, Method>(); + + private Map<String, Method> setMethods = new HashMap<String, Method>(); + + public ObjectMethods(Class clazz) + { + PropertyDescriptor[] descriptors = BeanUtils.getPropertyDescriptors(clazz); + + for (PropertyDescriptor descriptor : descriptors) + { + String fieldName = descriptor.getName().substring(0, 1).toUpperCase() + descriptor.getName().substring(1); + getMethods.put(fieldName, descriptor.getReadMethod()); + setMethods.put(fieldName, descriptor.getWriteMethod()); + } + } + + public Method getGetMethod(String fieldName) + { + return getMethods.get(fieldName); + } + + public Method getSetMethod(String fieldName) + { + return setMethods.get(fieldName); + } + + } + } -- GitLab