From 404d37cedcbbe9c2a0f8e9de90d95adc25859467 Mon Sep 17 00:00:00 2001 From: Frank Tao Date: Mon, 28 May 2018 22:45:45 -0400 Subject: [PATCH 1/5] Fixed #963 : load history table once instead of one by one in the loop in the SearchBuilder class. --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 24 +++++++++++++++---- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 6 ++--- .../main/java/ca/uhn/fhir/jpa/dao/IDao.java | 3 ++- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 11 +++++++-- .../dao/data/IResourceHistoryTableDao.java | 15 +++++++++--- 5 files changed, 46 insertions(+), 13 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 2e1713a3c19..b1004edf8c2 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1156,7 +1156,7 @@ public abstract class BaseHapiFhirDao implements IDao, public SearchBuilder newSearchBuilder() { SearchBuilder builder = new SearchBuilder(getContext(), myEntityManager, myFulltextSearchSvc, this, myResourceIndexedSearchParamUriDao, myForcedIdDao, - myTerminologySvc, mySerarchParamRegistry); + myTerminologySvc, mySerarchParamRegistry, myResourceHistoryTableDao); return builder; } @@ -1583,19 +1583,25 @@ public abstract class BaseHapiFhirDao implements IDao, public IBaseResource toResource(BaseHasResource theEntity, boolean theForHistoryOperation) { RuntimeResourceDefinition type = myContext.getResourceDefinition(theEntity.getResourceType()); Class resourceType = type.getImplementingClass(); - return toResource(resourceType, theEntity, theForHistoryOperation); + return toResource(resourceType, theEntity, null, theForHistoryOperation); } @SuppressWarnings("unchecked") @Override - public R toResource(Class theResourceType, BaseHasResource theEntity, + public R toResource(Class theResourceType, BaseHasResource theEntity, Collection historyList, boolean theForHistoryOperation) { + // May 28, 2018 - #936 + // Could set historyList to null, if it's not called in the loop for the backward compatibility ResourceHistoryTable history; if (theEntity instanceof ResourceHistoryTable) { history = (ResourceHistoryTable) theEntity; } else { - history = myResourceHistoryTableDao.findForIdAndVersion(theEntity.getId(), theEntity.getVersion()); + if (historyList == null) { + history = myResourceHistoryTableDao.findForIdAndVersion(theEntity.getId(), theEntity.getVersion()); + } else { + history = getHistory(historyList, theEntity.getId(), theEntity.getVersion()); + } } if (history == null) { @@ -1713,6 +1719,16 @@ public abstract class BaseHapiFhirDao implements IDao, } } + private ResourceHistoryTable getHistory(Collection historyList, Long resourceId, Long versionId) { + if (resourceId == null || versionId == null) + return null; + for (ResourceHistoryTable history : historyList) { + if (resourceId.equals(history.getResourceId()) && versionId.equals( history.getVersion())) + return history; + } + return null; + } + @SuppressWarnings("unchecked") protected ResourceTable updateEntity(RequestDetails theRequest, final IBaseResource theResource, ResourceTable theEntity, Date theDeletedTimestampOrNull, boolean thePerformIndexing, diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index c6c8228082d..574b4898f12 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -207,7 +207,7 @@ public abstract class BaseHapiFhirResourceDao extends B StopWatch w = new StopWatch(); - T resourceToDelete = toResource(myResourceType, entity, false); + T resourceToDelete = toResource(myResourceType, entity, null, false); // Notify IServerOperationInterceptors about pre-action call if (theReques != null) { @@ -289,7 +289,7 @@ public abstract class BaseHapiFhirResourceDao extends B ResourceTable entity = myEntityManager.find(ResourceTable.class, pid); deletedResources.add(entity); - T resourceToDelete = toResource(myResourceType, entity, false); + T resourceToDelete = toResource(myResourceType, entity, null, false); // Notify IServerOperationInterceptors about pre-action call if (theRequest != null) { @@ -854,7 +854,7 @@ public abstract class BaseHapiFhirResourceDao extends B BaseHasResource entity = readEntity(theId); validateResourceType(entity); - T retVal = toResource(myResourceType, entity, false); + T retVal = toResource(myResourceType, entity, null, false); IPrimitiveType deleted; if (retVal instanceof IResource) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java index 90f8e6e4a15..cfd84c9e0af 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java @@ -5,6 +5,7 @@ import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.entity.BaseHasResource; import ca.uhn.fhir.jpa.entity.ResourceTable; +import ca.uhn.fhir.jpa.entity.ResourceHistoryTable; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -56,6 +57,6 @@ public interface IDao { IBaseResource toResource(BaseHasResource theEntity, boolean theForHistoryOperation); - R toResource(Class theResourceType, BaseHasResource theEntity, boolean theForHistoryOperation); + R toResource(Class theResourceType, BaseHasResource theEntity, Collection historyList, boolean theForHistoryOperation); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index e208c64e1aa..a6badf819dd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -22,6 +22,7 @@ package ca.uhn.fhir.jpa.dao; import ca.uhn.fhir.context.*; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; +import ca.uhn.fhir.jpa.dao.data.IResourceHistoryTableDao; import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao; import ca.uhn.fhir.jpa.entity.*; import ca.uhn.fhir.jpa.search.JpaRuntimeSearchParam; @@ -107,12 +108,14 @@ public class SearchBuilder implements ISearchBuilder { private IHapiTerminologySvc myTerminologySvc; private int myFetchSize; + protected IResourceHistoryTableDao myResourceHistoryTableDao; /** * Constructor */ public SearchBuilder(FhirContext theFhirContext, EntityManager theEntityManager, IFulltextSearchSvc theFulltextSearchSvc, BaseHapiFhirDao theDao, - IResourceIndexedSearchParamUriDao theResourceIndexedSearchParamUriDao, IForcedIdDao theForcedIdDao, IHapiTerminologySvc theTerminologySvc, ISearchParamRegistry theSearchParamRegistry) { + IResourceIndexedSearchParamUriDao theResourceIndexedSearchParamUriDao, IForcedIdDao theForcedIdDao, IHapiTerminologySvc theTerminologySvc, ISearchParamRegistry theSearchParamRegistry, + IResourceHistoryTableDao theResourceHistoryTableDao) { myContext = theFhirContext; myEntityManager = theEntityManager; myFulltextSearchSvc = theFulltextSearchSvc; @@ -121,6 +124,7 @@ public class SearchBuilder implements ISearchBuilder { myForcedIdDao = theForcedIdDao; myTerminologySvc = theTerminologySvc; mySearchParamRegistry = theSearchParamRegistry; + myResourceHistoryTableDao = theResourceHistoryTableDao; } private void addPredicateComposite(String theResourceName, RuntimeSearchParam theParamDef, List theNextAnd) { @@ -1603,9 +1607,12 @@ public class SearchBuilder implements ISearchBuilder { List resultList = q.getResultList(); + //-- Issue #963: Load resource histories based on pids once to improve the performance + Collection historyList = myResourceHistoryTableDao.findByResourceIds(pids); + for (ResourceTable next : resultList) { Class resourceType = context.getResourceDefinition(next.getResourceType()).getImplementingClass(); - IBaseResource resource = theDao.toResource(resourceType, next, theForHistoryOperation); + IBaseResource resource = theDao.toResource(resourceType, next, historyList, theForHistoryOperation); if (resource == null) { ourLog.warn("Unable to find resource {}/{}/_history/{} in database", next.getResourceType(), next.getIdDt().getIdPart(), next.getVersion()); continue; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceHistoryTableDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceHistoryTableDao.java index d6250ce64f6..03fa39d7957 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceHistoryTableDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceHistoryTableDao.java @@ -1,6 +1,10 @@ package ca.uhn.fhir.jpa.dao.data; -import ca.uhn.fhir.jpa.entity.ResourceHistoryTable; +import java.util.Collection; +import java.util.Date; + +import javax.persistence.TemporalType; + import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; import org.springframework.data.jpa.repository.JpaRepository; @@ -8,8 +12,7 @@ import org.springframework.data.jpa.repository.Query; import org.springframework.data.jpa.repository.Temporal; import org.springframework.data.repository.query.Param; -import javax.persistence.TemporalType; -import java.util.Date; +import ca.uhn.fhir.jpa.entity.ResourceHistoryTable; /* * #%L @@ -82,4 +85,10 @@ public interface IResourceHistoryTableDao extends JpaRepository findIdsOfPreviousVersionsOfResources(Pageable thePage); + + @Query("" + + "SELECT h FROM ResourceHistoryTable h " + + "INNER JOIN ResourceTable r ON (r.myId = h.myResourceId and r.myVersion = h.myResourceVersion) " + + "WHERE r.myId in (:pids)") + Collection findByResourceIds(@Param("pids") Collection pids); } From 73f12744aee9cde99cc6a1ae754c963e63590af9 Mon Sep 17 00:00:00 2001 From: frankjtao <38163583+frankjtao@users.noreply.github.com> Date: Thu, 31 May 2018 21:30:46 -0400 Subject: [PATCH 2/5] #963 Improved performance for tag loading --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 43 ++-- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 6 +- .../main/java/ca/uhn/fhir/jpa/dao/IDao.java | 14 +- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 207 ++++++++++++++---- .../fhir/jpa/dao/data/IResourceTagDao.java | 10 +- 5 files changed, 208 insertions(+), 72 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index b1004edf8c2..71492da936b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1156,7 +1156,7 @@ public abstract class BaseHapiFhirDao implements IDao, public SearchBuilder newSearchBuilder() { SearchBuilder builder = new SearchBuilder(getContext(), myEntityManager, myFulltextSearchSvc, this, myResourceIndexedSearchParamUriDao, myForcedIdDao, - myTerminologySvc, mySerarchParamRegistry, myResourceHistoryTableDao); + myTerminologySvc, mySerarchParamRegistry, myResourceHistoryTableDao, myResourceTagDao); return builder; } @@ -1337,7 +1337,7 @@ public abstract class BaseHapiFhirDao implements IDao, } @SuppressWarnings("unchecked") - private R populateResourceMetadataHapi(Class theResourceType, BaseHasResource theEntity, boolean theForHistoryOperation, IResource res) { + private R populateResourceMetadataHapi(Class theResourceType, BaseHasResource theEntity, Collection myTagList, boolean theForHistoryOperation, IResource res) { R retVal = (R) res; if (theEntity.getDeleted() != null) { res = (IResource) myContext.getResourceDefinition(theResourceType).newInstance(); @@ -1366,7 +1366,7 @@ public abstract class BaseHapiFhirDao implements IDao, ResourceMetadataKeyEnum.UPDATED.put(res, theEntity.getUpdated()); IDao.RESOURCE_PID.put(res, theEntity.getId()); - Collection tags = theEntity.getTags(); + Collection tags = myTagList; if (theEntity.isHasTags()) { TagList tagList = new TagList(); List securityLabels = new ArrayList<>(); @@ -1403,7 +1403,7 @@ public abstract class BaseHapiFhirDao implements IDao, } @SuppressWarnings("unchecked") - private R populateResourceMetadataRi(Class theResourceType, BaseHasResource theEntity, boolean theForHistoryOperation, IAnyResource res) { + private R populateResourceMetadataRi(Class theResourceType, BaseHasResource theEntity, Collection myTagList, boolean theForHistoryOperation, IAnyResource res) { R retVal = (R) res; if (theEntity.getDeleted() != null) { res = (IAnyResource) myContext.getResourceDefinition(theResourceType).newInstance(); @@ -1436,7 +1436,7 @@ public abstract class BaseHapiFhirDao implements IDao, res.getMeta().setLastUpdated(theEntity.getUpdatedDate()); IDao.RESOURCE_PID.put(res, theEntity.getId()); - Collection tags = theEntity.getTags(); + Collection tags = myTagList; if (theEntity.isHasTags()) { for (BaseTag next : tags) { @@ -1583,12 +1583,12 @@ public abstract class BaseHapiFhirDao implements IDao, public IBaseResource toResource(BaseHasResource theEntity, boolean theForHistoryOperation) { RuntimeResourceDefinition type = myContext.getResourceDefinition(theEntity.getResourceType()); Class resourceType = type.getImplementingClass(); - return toResource(resourceType, theEntity, null, theForHistoryOperation); + return toResource(resourceType, theEntity, null, null, theForHistoryOperation); } @SuppressWarnings("unchecked") @Override - public R toResource(Class theResourceType, BaseHasResource theEntity, Collection historyList, + public R toResource(Class theResourceType, BaseHasResource theEntity, ResourceHistoryTable myHistory, Collection tagList, boolean theForHistoryOperation) { // May 28, 2018 - #936 @@ -1597,10 +1597,10 @@ public abstract class BaseHapiFhirDao implements IDao, if (theEntity instanceof ResourceHistoryTable) { history = (ResourceHistoryTable) theEntity; } else { - if (historyList == null) { + if (myHistory == null) { history = myResourceHistoryTableDao.findForIdAndVersion(theEntity.getId(), theEntity.getVersion()); } else { - history = getHistory(historyList, theEntity.getId(), theEntity.getVersion()); + history = myHistory; } } @@ -1627,12 +1627,21 @@ public abstract class BaseHapiFhirDao implements IDao, break; } + // get preload the tagList + Collection myTagList; + + if (tagList == null) + myTagList = theEntity.getTags(); + else + myTagList = tagList; + + /* * Use the appropriate custom type if one is specified in the context */ Class resourceType = theResourceType; if (myContext.hasDefaultTypeForProfile()) { - for (BaseTag nextTag : theEntity.getTags()) { + for (BaseTag nextTag : myTagList) { if (nextTag.getTag().getTagType() == TagTypeEnum.PROFILE) { String profile = nextTag.getTag().getCode(); if (isNotBlank(profile)) { @@ -1679,10 +1688,10 @@ public abstract class BaseHapiFhirDao implements IDao, if (retVal instanceof IResource) { IResource res = (IResource) retVal; - retVal = populateResourceMetadataHapi(resourceType, theEntity, theForHistoryOperation, res); + retVal = populateResourceMetadataHapi(resourceType, theEntity, myTagList, theForHistoryOperation, res); } else { IAnyResource res = (IAnyResource) retVal; - retVal = populateResourceMetadataRi(resourceType, theEntity, theForHistoryOperation, res); + retVal = populateResourceMetadataRi(resourceType, theEntity, myTagList, theForHistoryOperation, res); } @@ -1718,16 +1727,6 @@ public abstract class BaseHapiFhirDao implements IDao, return theResourceType + '/' + theId.toString(); } } - - private ResourceHistoryTable getHistory(Collection historyList, Long resourceId, Long versionId) { - if (resourceId == null || versionId == null) - return null; - for (ResourceHistoryTable history : historyList) { - if (resourceId.equals(history.getResourceId()) && versionId.equals( history.getVersion())) - return history; - } - return null; - } @SuppressWarnings("unchecked") protected ResourceTable updateEntity(RequestDetails theRequest, final IBaseResource theResource, ResourceTable diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java index 574b4898f12..6ee37b53db9 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java @@ -207,7 +207,7 @@ public abstract class BaseHapiFhirResourceDao extends B StopWatch w = new StopWatch(); - T resourceToDelete = toResource(myResourceType, entity, null, false); + T resourceToDelete = toResource(myResourceType, entity, null, null, false); // Notify IServerOperationInterceptors about pre-action call if (theReques != null) { @@ -289,7 +289,7 @@ public abstract class BaseHapiFhirResourceDao extends B ResourceTable entity = myEntityManager.find(ResourceTable.class, pid); deletedResources.add(entity); - T resourceToDelete = toResource(myResourceType, entity, null, false); + T resourceToDelete = toResource(myResourceType, entity, null, null, false); // Notify IServerOperationInterceptors about pre-action call if (theRequest != null) { @@ -854,7 +854,7 @@ public abstract class BaseHapiFhirResourceDao extends B BaseHasResource entity = readEntity(theId); validateResourceType(entity); - T retVal = toResource(myResourceType, entity, null, false); + T retVal = toResource(myResourceType, entity, null, null, false); IPrimitiveType deleted; if (retVal instanceof IResource) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java index cfd84c9e0af..9cb341b2bc1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java @@ -1,16 +1,18 @@ package ca.uhn.fhir.jpa.dao; +import java.util.Collection; +import java.util.Set; + +import org.hl7.fhir.instance.model.api.IBaseResource; + import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.entity.BaseHasResource; -import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.entity.ResourceHistoryTable; +import ca.uhn.fhir.jpa.entity.ResourceTable; +import ca.uhn.fhir.jpa.entity.ResourceTag; import ca.uhn.fhir.jpa.search.PersistedJpaBundleProvider; -import org.hl7.fhir.instance.model.api.IBaseResource; - -import java.util.Collection; -import java.util.Set; /* * #%L @@ -57,6 +59,6 @@ public interface IDao { IBaseResource toResource(BaseHasResource theEntity, boolean theForHistoryOperation); - R toResource(Class theResourceType, BaseHasResource theEntity, Collection historyList, boolean theForHistoryOperation); + R toResource(Class theResourceType, BaseHasResource theEntity, ResourceHistoryTable myHistory, Collection tagList, boolean theForHistoryOperation); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java index a6badf819dd..6d4d27ae5d8 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java @@ -1,5 +1,58 @@ package ca.uhn.fhir.jpa.dao; +import static org.apache.commons.lang3.StringUtils.defaultString; +import static org.apache.commons.lang3.StringUtils.isBlank; +import static org.apache.commons.lang3.StringUtils.isNotBlank; + +import java.math.BigDecimal; +import java.math.MathContext; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; + +import javax.persistence.EntityManager; +import javax.persistence.TypedQuery; +import javax.persistence.criteria.AbstractQuery; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaBuilder.In; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Expression; +import javax.persistence.criteria.From; +import javax.persistence.criteria.Join; +import javax.persistence.criteria.JoinType; +import javax.persistence.criteria.Order; +import javax.persistence.criteria.Path; +import javax.persistence.criteria.Predicate; +import javax.persistence.criteria.Root; +import javax.persistence.criteria.Subquery; + +import org.apache.commons.lang3.ObjectUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Validate; +import org.apache.commons.lang3.builder.EqualsBuilder; +import org.apache.commons.lang3.builder.HashCodeBuilder; +import org.apache.commons.lang3.tuple.Pair; +import org.hibernate.ScrollMode; +import org.hibernate.ScrollableResults; +import org.hibernate.query.Query; +import org.hl7.fhir.dstu3.model.BaseResource; +import org.hl7.fhir.instance.model.api.IAnyResource; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IIdType; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; + /* * #%L * HAPI FHIR JPA Server @@ -19,18 +72,45 @@ package ca.uhn.fhir.jpa.dao; * limitations under the License. * #L% */ - -import ca.uhn.fhir.context.*; +import ca.uhn.fhir.context.BaseRuntimeChildDefinition; +import ca.uhn.fhir.context.BaseRuntimeDeclaredChildDefinition; +import ca.uhn.fhir.context.BaseRuntimeElementDefinition; +import ca.uhn.fhir.context.ConfigurationException; +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.context.RuntimeChildChoiceDefinition; +import ca.uhn.fhir.context.RuntimeChildResourceDefinition; +import ca.uhn.fhir.context.RuntimeResourceDefinition; +import ca.uhn.fhir.context.RuntimeSearchParam; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IResourceHistoryTableDao; import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao; -import ca.uhn.fhir.jpa.entity.*; +import ca.uhn.fhir.jpa.dao.data.IResourceTagDao; +import ca.uhn.fhir.jpa.entity.BaseHasResource; +import ca.uhn.fhir.jpa.entity.BaseResourceIndexedSearchParam; +import ca.uhn.fhir.jpa.entity.ResourceHistoryTable; +import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamDate; +import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamNumber; +import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamQuantity; +import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamString; +import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamToken; +import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamUri; +import ca.uhn.fhir.jpa.entity.ResourceLink; +import ca.uhn.fhir.jpa.entity.ResourceTable; +import ca.uhn.fhir.jpa.entity.ResourceTag; +import ca.uhn.fhir.jpa.entity.SearchParam; +import ca.uhn.fhir.jpa.entity.SearchParamPresent; +import ca.uhn.fhir.jpa.entity.TagDefinition; +import ca.uhn.fhir.jpa.entity.TagTypeEnum; import ca.uhn.fhir.jpa.search.JpaRuntimeSearchParam; import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.term.VersionIndependentConcept; import ca.uhn.fhir.jpa.util.BaseIterator; import ca.uhn.fhir.jpa.util.ScrollableResultsIterator; -import ca.uhn.fhir.model.api.*; +import ca.uhn.fhir.model.api.IPrimitiveDatatype; +import ca.uhn.fhir.model.api.IQueryParameterType; +import ca.uhn.fhir.model.api.IResource; +import ca.uhn.fhir.model.api.Include; +import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.model.base.composite.BaseCodingDt; import ca.uhn.fhir.model.base.composite.BaseIdentifierDt; import ca.uhn.fhir.model.base.composite.BaseQuantityDt; @@ -42,41 +122,25 @@ import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum; import ca.uhn.fhir.rest.api.SortOrderEnum; import ca.uhn.fhir.rest.api.SortSpec; -import ca.uhn.fhir.rest.param.*; +import ca.uhn.fhir.rest.param.CompositeParam; +import ca.uhn.fhir.rest.param.DateParam; +import ca.uhn.fhir.rest.param.DateRangeParam; +import ca.uhn.fhir.rest.param.HasParam; +import ca.uhn.fhir.rest.param.NumberParam; +import ca.uhn.fhir.rest.param.ParamPrefixEnum; +import ca.uhn.fhir.rest.param.QuantityParam; +import ca.uhn.fhir.rest.param.ReferenceParam; +import ca.uhn.fhir.rest.param.StringParam; +import ca.uhn.fhir.rest.param.TokenParam; +import ca.uhn.fhir.rest.param.TokenParamModifier; +import ca.uhn.fhir.rest.param.UriParam; +import ca.uhn.fhir.rest.param.UriParamQualifierEnum; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.util.StopWatch; import ca.uhn.fhir.util.UrlUtil; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; -import org.apache.commons.lang3.ObjectUtils; -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.lang3.Validate; -import org.apache.commons.lang3.builder.EqualsBuilder; -import org.apache.commons.lang3.builder.HashCodeBuilder; -import org.apache.commons.lang3.tuple.Pair; -import org.hibernate.ScrollMode; -import org.hibernate.ScrollableResults; -import org.hibernate.query.Query; -import org.hl7.fhir.dstu3.model.BaseResource; -import org.hl7.fhir.instance.model.api.IAnyResource; -import org.hl7.fhir.instance.model.api.IBaseResource; -import org.hl7.fhir.instance.model.api.IIdType; - -import javax.persistence.EntityManager; -import javax.persistence.TypedQuery; -import javax.persistence.criteria.*; -import javax.persistence.criteria.CriteriaBuilder.In; -import java.math.BigDecimal; -import java.math.MathContext; -import java.util.*; -import java.util.Map.Entry; - -import static org.apache.commons.lang3.StringUtils.*; /** * The SearchBuilder is responsible for actually forming the SQL query that handles @@ -109,13 +173,16 @@ public class SearchBuilder implements ISearchBuilder { private int myFetchSize; protected IResourceHistoryTableDao myResourceHistoryTableDao; + protected IResourceTagDao myResourceTagDao; + /** * Constructor */ - public SearchBuilder(FhirContext theFhirContext, EntityManager theEntityManager, IFulltextSearchSvc theFulltextSearchSvc, - BaseHapiFhirDao theDao, - IResourceIndexedSearchParamUriDao theResourceIndexedSearchParamUriDao, IForcedIdDao theForcedIdDao, IHapiTerminologySvc theTerminologySvc, ISearchParamRegistry theSearchParamRegistry, - IResourceHistoryTableDao theResourceHistoryTableDao) { + public SearchBuilder(FhirContext theFhirContext, EntityManager theEntityManager, + IFulltextSearchSvc theFulltextSearchSvc, BaseHapiFhirDao theDao, + IResourceIndexedSearchParamUriDao theResourceIndexedSearchParamUriDao, IForcedIdDao theForcedIdDao, + IHapiTerminologySvc theTerminologySvc, ISearchParamRegistry theSearchParamRegistry, + IResourceHistoryTableDao theResourceHistoryTableDao, IResourceTagDao theResourceTagDao) { myContext = theFhirContext; myEntityManager = theEntityManager; myFulltextSearchSvc = theFulltextSearchSvc; @@ -125,6 +192,7 @@ public class SearchBuilder implements ISearchBuilder { myTerminologySvc = theTerminologySvc; mySearchParamRegistry = theSearchParamRegistry; myResourceHistoryTableDao = theResourceHistoryTableDao; + myResourceTagDao = theResourceTagDao; } private void addPredicateComposite(String theResourceName, RuntimeSearchParam theParamDef, List theNextAnd) { @@ -1608,11 +1676,14 @@ public class SearchBuilder implements ISearchBuilder { List resultList = q.getResultList(); //-- Issue #963: Load resource histories based on pids once to improve the performance - Collection historyList = myResourceHistoryTableDao.findByResourceIds(pids); - + Map historyMap = getResourceHistoryMap(pids); + + //-- preload all tags with tag definition if any + Map> tagMap = getResourceTagMap(resultList); + for (ResourceTable next : resultList) { Class resourceType = context.getResourceDefinition(next.getResourceType()).getImplementingClass(); - IBaseResource resource = theDao.toResource(resourceType, next, historyList, theForHistoryOperation); + IBaseResource resource = theDao.toResource(resourceType, next, historyMap.get(next.getId()), tagMap.get(next.getId()), theForHistoryOperation); if (resource == null) { ourLog.warn("Unable to find resource {}/{}/_history/{} in database", next.getResourceType(), next.getIdDt().getIdPart(), next.getVersion()); continue; @@ -1641,6 +1712,62 @@ public class SearchBuilder implements ISearchBuilder { } } + //-- load all history in to the map + private Map getResourceHistoryMap(Collection pids) { + + Map historyMap = new HashMap(); + + if (pids.size() == 0) + return historyMap; + + Collection historyList = myResourceHistoryTableDao.findByResourceIds(pids); + + for (ResourceHistoryTable history : historyList) { + + historyMap.put(history.getResourceId(), history); + } + + return historyMap; + } + + private Map> getResourceTagMap(List resourceList) { + + List idList = new ArrayList(resourceList.size()); + + //-- find all resource has tags + for (ResourceTable resource: resourceList) { + if (resource.isHasTags()) + idList.add(resource.getId()); + } + + Map> tagMap = new HashMap>(); + + //-- no tags + if (idList.size() == 0) + return tagMap; + + //-- get all tags for the idList + Collection tagList = myResourceTagDao.findByResourceIds(idList); + + //-- build the map, key = resourceId, value = list of ResourceTag + Long resourceId; + Collection tagCol; + for (ResourceTag tag : tagList) { + + resourceId = tag.getResourceId(); + tagCol = tagMap.get(resourceId); + if (tagCol == null) { + tagCol = new ArrayList(); + tagCol.add(tag); + tagMap.put(resourceId, tagCol); + } else { + tagCol.add(tag); + } + } + + return tagMap; + } + @Override public void loadResourcesByPid(Collection theIncludePids, List theResourceListToPopulate, Set theRevIncludedPids, boolean theForHistoryOperation, EntityManager entityManager, FhirContext context, IDao theDao) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTagDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTagDao.java index 3acba7469a6..1ba407c1f4f 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTagDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTagDao.java @@ -1,5 +1,7 @@ package ca.uhn.fhir.jpa.dao.data; +import java.util.Collection; + /* * #%L * HAPI FHIR JPA Server @@ -21,9 +23,15 @@ package ca.uhn.fhir.jpa.dao.data; */ import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import ca.uhn.fhir.jpa.entity.ResourceTag; public interface IResourceTagDao extends JpaRepository { - // nothing + @Query("" + + "SELECT t FROM ResourceTag t " + + "INNER JOIN TagDefinition td ON (td.myId = t.myTagId) " + + "WHERE t.myResourceId in (:pids)") + Collection findByResourceIds(@Param("pids") Collection pids); } From 35991cd21bae09df2998579939009272c8612943 Mon Sep 17 00:00:00 2001 From: frankjtao <38163583+frankjtao@users.noreply.github.com> Date: Thu, 7 Jun 2018 14:19:44 -0400 Subject: [PATCH 3/5] #963: renamed the parameter to 'the*' --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 18 +++++++++--------- .../main/java/ca/uhn/fhir/jpa/dao/IDao.java | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 71492da936b..6ed32a432be 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1337,7 +1337,7 @@ public abstract class BaseHapiFhirDao implements IDao, } @SuppressWarnings("unchecked") - private R populateResourceMetadataHapi(Class theResourceType, BaseHasResource theEntity, Collection myTagList, boolean theForHistoryOperation, IResource res) { + private R populateResourceMetadataHapi(Class theResourceType, BaseHasResource theEntity, Collection theTagList, boolean theForHistoryOperation, IResource res) { R retVal = (R) res; if (theEntity.getDeleted() != null) { res = (IResource) myContext.getResourceDefinition(theResourceType).newInstance(); @@ -1366,7 +1366,7 @@ public abstract class BaseHapiFhirDao implements IDao, ResourceMetadataKeyEnum.UPDATED.put(res, theEntity.getUpdated()); IDao.RESOURCE_PID.put(res, theEntity.getId()); - Collection tags = myTagList; + Collection tags = theTagList; if (theEntity.isHasTags()) { TagList tagList = new TagList(); List securityLabels = new ArrayList<>(); @@ -1403,7 +1403,7 @@ public abstract class BaseHapiFhirDao implements IDao, } @SuppressWarnings("unchecked") - private R populateResourceMetadataRi(Class theResourceType, BaseHasResource theEntity, Collection myTagList, boolean theForHistoryOperation, IAnyResource res) { + private R populateResourceMetadataRi(Class theResourceType, BaseHasResource theEntity, Collection theTagList, boolean theForHistoryOperation, IAnyResource res) { R retVal = (R) res; if (theEntity.getDeleted() != null) { res = (IAnyResource) myContext.getResourceDefinition(theResourceType).newInstance(); @@ -1436,7 +1436,7 @@ public abstract class BaseHapiFhirDao implements IDao, res.getMeta().setLastUpdated(theEntity.getUpdatedDate()); IDao.RESOURCE_PID.put(res, theEntity.getId()); - Collection tags = myTagList; + Collection tags = theTagList; if (theEntity.isHasTags()) { for (BaseTag next : tags) { @@ -1588,7 +1588,7 @@ public abstract class BaseHapiFhirDao implements IDao, @SuppressWarnings("unchecked") @Override - public R toResource(Class theResourceType, BaseHasResource theEntity, ResourceHistoryTable myHistory, Collection tagList, + public R toResource(Class theResourceType, BaseHasResource theEntity, ResourceHistoryTable theHistory, Collection theTagList, boolean theForHistoryOperation) { // May 28, 2018 - #936 @@ -1597,10 +1597,10 @@ public abstract class BaseHapiFhirDao implements IDao, if (theEntity instanceof ResourceHistoryTable) { history = (ResourceHistoryTable) theEntity; } else { - if (myHistory == null) { + if (theHistory == null) { history = myResourceHistoryTableDao.findForIdAndVersion(theEntity.getId(), theEntity.getVersion()); } else { - history = myHistory; + history = theHistory; } } @@ -1630,10 +1630,10 @@ public abstract class BaseHapiFhirDao implements IDao, // get preload the tagList Collection myTagList; - if (tagList == null) + if (theTagList == null) myTagList = theEntity.getTags(); else - myTagList = tagList; + myTagList = theTagList; /* diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java index 9cb341b2bc1..1a389835ef1 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/IDao.java @@ -59,6 +59,6 @@ public interface IDao { IBaseResource toResource(BaseHasResource theEntity, boolean theForHistoryOperation); - R toResource(Class theResourceType, BaseHasResource theEntity, ResourceHistoryTable myHistory, Collection tagList, boolean theForHistoryOperation); + R toResource(Class theResourceType, BaseHasResource theEntity, ResourceHistoryTable theHistory, Collection theTagList, boolean theForHistoryOperation); } From 299cf5b230b1dee4b9848747bfcc7dcb95ef7863 Mon Sep 17 00:00:00 2001 From: frankjtao <38163583+frankjtao@users.noreply.github.com> Date: Sat, 16 Jun 2018 22:37:12 -0400 Subject: [PATCH 4/5] Changed retval to resource id instead of SearchResult in SearchResultDao --- .../ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java | 4 ++-- .../uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java | 6 +++--- .../fhir/jpa/search/SearchCoordinatorSvcImplTest.java | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java index 12649857f48..4f7e05d9131 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/ISearchResultDao.java @@ -38,8 +38,8 @@ public interface ISearchResultDao extends JpaRepository { @Query(value="SELECT r FROM SearchResult r WHERE r.mySearch = :search") Collection findWithSearchUuid(@Param("search") Search theSearch); - @Query(value="SELECT r FROM SearchResult r WHERE r.mySearch = :search ORDER BY r.myOrder ASC") - Page findWithSearchUuid(@Param("search") Search theSearch, Pageable thePage); + @Query(value="SELECT r.myResourcePid FROM SearchResult r WHERE r.mySearch = :search ORDER BY r.myOrder ASC") + Page findWithSearchUuid(@Param("search") Search theSearch, Pageable thePage); @Modifying @Query(value="DELETE FROM SearchResult r WHERE r.mySearchPid = :search") diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java index dcace7b4fc2..1dcd22462f0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImpl.java @@ -178,9 +178,9 @@ public class SearchCoordinatorSvcImpl implements ISearchCoordinatorSvc { @Override public List doInTransaction(TransactionStatus theStatus) { final List resultPids = new ArrayList(); - Page searchResults = mySearchResultDao.findWithSearchUuid(foundSearch, page); - for (SearchResult next : searchResults) { - resultPids.add(next.getResourcePid()); + Page searchResultPids = mySearchResultDao.findWithSearchUuid(foundSearch, page); + for (Long next : searchResultPids) { + resultPids.add(next); } return resultPids; } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java index 9126cec246a..07316c53160 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java @@ -323,18 +323,18 @@ public class SearchCoordinatorSvcImplTest { // ignore } - when(mySearchResultDao.findWithSearchUuid(any(Search.class), any(Pageable.class))).thenAnswer(new Answer>() { + when(mySearchResultDao.findWithSearchUuid(any(Search.class), any(Pageable.class))).thenAnswer(new Answer>() { @Override - public Page answer(InvocationOnMock theInvocation) throws Throwable { + public Page answer(InvocationOnMock theInvocation) throws Throwable { Pageable page = (Pageable) theInvocation.getArguments()[1]; - ArrayList results = new ArrayList(); + ArrayList results = new ArrayList(); int max = (page.getPageNumber() * page.getPageSize()) + page.getPageSize(); for (int i = page.getOffset(); i < max; i++) { - results.add(new SearchResult().setResourcePid(i + 10L)); + results.add(i + 10L); } - return new PageImpl(results); + return new PageImpl(results); } }); search.setStatus(SearchStatusEnum.FINISHED); From 6e7d67ba7090f73f260b424bf8f50bdb7812bb98 Mon Sep 17 00:00:00 2001 From: frankjtao <38163583+frankjtao@users.noreply.github.com> Date: Tue, 19 Jun 2018 20:16:27 -0400 Subject: [PATCH 5/5] Update due to Spring version change --- .../ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java index 625fbac15ae..90b3c320e07 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/search/SearchCoordinatorSvcImplTest.java @@ -328,7 +328,7 @@ public class SearchCoordinatorSvcImplTest { ArrayList results = new ArrayList(); int max = (page.getPageNumber() * page.getPageSize()) + page.getPageSize(); - for (int i = page.getOffset(); i < max; i++) { + for (long i = page.getOffset(); i < max; i++) { results.add(i + 10L); }