From 69849dd3c5c79ce741a2d7816f4251cb372eade2 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Wed, 14 Jun 2017 08:35:41 -0400 Subject: [PATCH 1/8] Optimize queries in JPA --- hapi-fhir-jpaserver-base/pom.xml | 5 + .../fhir/jpa/dao/BaseHapiFhirSystemDao.java | 33 +- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 355 +++++++++--------- .../fhir/jpa/dao/data/IResourceTableDao.java | 10 +- .../uhn/fhir/jpa/config/TestDstu3Config.java | 13 +- .../FhirResourceDaoDstu3SearchNoFtTest.java | 41 ++ pom.xml | 5 + src/changes/changes.xml | 4 + 8 files changed, 273 insertions(+), 193 deletions(-) diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index 6008c0f9aa8..365b4a34a89 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -100,6 +100,11 @@ thymeleaf test + + net.ttddyy + datasource-proxy + test + org.javassist diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java index 051ee0bdeb7..5ed0a47b7f0 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java @@ -33,6 +33,7 @@ import javax.persistence.criteria.Root; import org.hl7.fhir.instance.model.api.IBaseResource; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.PageRequest; import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.annotation.Propagation; @@ -40,8 +41,7 @@ import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionCallback; import org.springframework.transaction.support.TransactionTemplate; -import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; -import ca.uhn.fhir.jpa.dao.data.ITermConceptDao; +import ca.uhn.fhir.jpa.dao.data.*; import ca.uhn.fhir.jpa.entity.ForcedId; import ca.uhn.fhir.jpa.entity.ResourceTable; import ca.uhn.fhir.jpa.util.ReindexFailureException; @@ -87,30 +87,33 @@ public abstract class BaseHapiFhirSystemDao extends BaseHapiFhirDao() { @SuppressWarnings("unchecked") @Override public Integer doInTransaction(TransactionStatus theStatus) { - TypedQuery q = myEntityManager.createQuery("SELECT t FROM " + ResourceTable.class.getSimpleName() + " t WHERE t.myIndexStatus IS null", ResourceTable.class); int maxResult = 500; if (theCount != null) { maxResult = Math.min(theCount, 2000); } + maxResult = Math.max(maxResult, 10); + TypedQuery q = myEntityManager.createQuery("SELECT t.myId FROM ResourceTable t WHERE t.myIndexStatus IS NULL", Long.class); + + ourLog.info("Beginning indexing query with maximum {}", maxResult); q.setMaxResults(maxResult); - List resources = q.getResultList(); - if (resources.isEmpty()) { - return 0; - } - - ourLog.info("Indexing {} resources", resources.size()); + Collection resources = q.getResultList(); int count = 0; long start = System.currentTimeMillis(); - for (ResourceTable resourceTable : resources) { + for (Long nextId : resources) { + ResourceTable resourceTable = myResourceTableDao.findOne(nextId); + try { /* * This part is because from HAPI 1.5 - 1.6 we changed the format of forced ID to be "type/id" instead of just "id" @@ -135,13 +138,17 @@ public abstract class BaseHapiFhirSystemDao extends BaseHapiFhirDao= maxResult) { + break; + } } long delay = System.currentTimeMillis() - start; - long avg = (delay / resources.size()); - ourLog.info("Indexed {} / {} resources in {}ms - Avg {}ms / resource", new Object[] { count, resources.size(), delay, avg }); + long avg = count > 0 ? (delay / count) : 0; + ourLog.info("Indexed {} resources in {}ms - Avg {}ms / resource", new Object[] { count, delay, avg }); - return resources.size(); + return count; } }); } 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 7de8452ee3b..110d0703276 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 @@ -27,90 +27,31 @@ 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.*; 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.*; 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.*; +import org.apache.commons.lang3.builder.EqualsBuilder; +import org.apache.commons.lang3.builder.HashCodeBuilder; import org.apache.commons.lang3.tuple.Pair; -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 org.hl7.fhir.instance.model.api.*; -import com.google.common.collect.Lists; -import com.google.common.collect.Sets; +import com.google.common.collect.*; -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.FhirVersionEnum; -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.BaseHapiFhirDao; -import ca.uhn.fhir.jpa.dao.BaseHapiFhirResourceDao; -import ca.uhn.fhir.jpa.dao.IDao; -import ca.uhn.fhir.jpa.dao.IFhirResourceDao; -import ca.uhn.fhir.jpa.dao.IFulltextSearchSvc; -import ca.uhn.fhir.jpa.dao.ISearchBuilder; -import ca.uhn.fhir.jpa.dao.ISearchParamRegistry; +import ca.uhn.fhir.context.*; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao; -import ca.uhn.fhir.jpa.entity.BaseHasResource; -import ca.uhn.fhir.jpa.entity.BaseResourceIndexedSearchParam; -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.entity.*; import ca.uhn.fhir.jpa.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.term.VersionIndependentConcept; import ca.uhn.fhir.jpa.util.StopWatch; -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; +import ca.uhn.fhir.model.api.*; +import ca.uhn.fhir.model.base.composite.*; import ca.uhn.fhir.model.dstu.resource.BaseResource; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.primitive.InstantDt; @@ -119,23 +60,9 @@ import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.rest.api.SortOrderEnum; import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.method.RestSearchParameterTypeEnum; -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.param.*; import ca.uhn.fhir.rest.server.Constants; -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; -import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; +import ca.uhn.fhir.rest.server.exceptions.*; import ca.uhn.fhir.util.UrlUtil; /** @@ -145,12 +72,14 @@ import ca.uhn.fhir.util.UrlUtil; public class SearchBuilder implements ISearchBuilder { private static Long NO_MORE = Long.valueOf(-1); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchBuilder.class); + private List myAlsoIncludePids; private CriteriaBuilder myBuilder; private BaseHapiFhirDao myCallingDao; private FhirContext myContext; private EntityManager myEntityManager; private IForcedIdDao myForcedIdDao; private IFulltextSearchSvc myFulltextSearchSvc; + private Map> myIndexJoins = Maps.newHashMap(); private SearchParameterMap myParams; private ArrayList myPredicates; private IResourceIndexedSearchParamUriDao myResourceIndexedSearchParamUriDao; @@ -159,8 +88,8 @@ public class SearchBuilder implements ISearchBuilder { private Root myResourceTableRoot; private Class myResourceType; private ISearchParamRegistry mySearchParamRegistry; - private IHapiTerminologySvc myTerminologySvc; private String mySearchUuid; + private IHapiTerminologySvc myTerminologySvc; /** * Constructor @@ -199,7 +128,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateDate(String theResourceName, String theParamName, List theList) { - Join join = myResourceTableRoot.join("myParamsDate", JoinType.LEFT); + Join join = createOrReuseJoin(JoinEnum.DATE, theParamName); if (theList.get(0).getMissing() != null) { Boolean missing = theList.get(0).getMissing(); @@ -275,28 +204,6 @@ public class SearchBuilder implements ISearchBuilder { } } - // private void addPredicateId(Set thePids) { - // if (thePids == null || thePids.isEmpty()) { - // return; - // } - // - // CriteriaBuilder builder = myEntityManager.getCriteriaBuilder(); - // CriteriaQuery cq = builder.createQuery(Long.class); - // Root from = cq.from(ResourceTable.class); - // cq.select(from.get("myId").as(Long.class)); - // - // List predicates = new ArrayList(); - // predicates.add(builder.equal(from.get("myResourceType"), myResourceName)); - // predicates.add(from.get("myId").in(thePids)); - // createPredicateResourceId(builder, cq, predicates, from.get("myId").as(Long.class)); - // createPredicateLastUpdatedForResourceTable(builder, from, predicates); - // - // cq.where(toArray(predicates)); - // - // TypedQuery q = myEntityManager.createQuery(cq); - // doSetPids(q.getResultList()); - // } - private void addPredicateLanguage(List> theList) { for (List nextList : theList) { @@ -326,7 +233,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateNumber(String theResourceName, String theParamName, List theList) { - Join join = myResourceTableRoot.join("myParamsNumber", JoinType.LEFT); + Join join = createOrReuseJoin(JoinEnum.NUMBER, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -378,7 +285,7 @@ public class SearchBuilder implements ISearchBuilder { } private void addPredicateQuantity(String theResourceName, String theParamName, List theList) { - Join join = myResourceTableRoot.join("myParamsQuantity", JoinType.LEFT); + Join join = createOrReuseJoin(JoinEnum.QUANTITY, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -406,7 +313,7 @@ public class SearchBuilder implements ISearchBuilder { return; } - Join join = myResourceTableRoot.join("myResourceLinks", JoinType.LEFT); + Join join = createOrReuseJoin(JoinEnum.REFERENCE, theParamName); List codePredicates = new ArrayList(); @@ -466,7 +373,7 @@ public class SearchBuilder implements ISearchBuilder { RuntimeResourceDefinition resourceDef = myContext.getResourceDefinition(theResourceName); RuntimeSearchParam searchParamByName = myCallingDao.getSearchParamByName(resourceDef, theParamName); if (searchParamByName == null) { - throw new InternalErrorException("Could not find parameter " + theParamName ); + throw new InternalErrorException("Could not find parameter " + theParamName); } String paramPath = searchParamByName.getPath(); if (paramPath.endsWith(".as(Reference)")) { @@ -496,7 +403,7 @@ public class SearchBuilder implements ISearchBuilder { if (resourceTypes.isEmpty()) { for (BaseRuntimeElementDefinition next : myContext.getElementDefinitions()) { if (next instanceof RuntimeResourceDefinition) { - RuntimeResourceDefinition nextResDef = (RuntimeResourceDefinition)next; + RuntimeResourceDefinition nextResDef = (RuntimeResourceDefinition) next; resourceTypes.add(nextResDef.getImplementingClass()); } } @@ -582,8 +489,10 @@ public class SearchBuilder implements ISearchBuilder { */ Root stackRoot = myResourceTableRoot; ArrayList stackPredicates = myPredicates; + Map> stackIndexJoins = myIndexJoins; myResourceTableRoot = subQfrom; - myPredicates = new ArrayList(); + myPredicates = Lists.newArrayList(); + myIndexJoins = Maps.newHashMap(); // Create the subquery predicates myPredicates.add(myBuilder.equal(myResourceTableRoot.get("myResourceType"), subResourceName)); @@ -597,6 +506,7 @@ public class SearchBuilder implements ISearchBuilder { */ myResourceTableRoot = stackRoot; myPredicates = stackPredicates; + myIndexJoins = stackIndexJoins; Predicate pathPredicate = createResourceLinkPathPredicate(theResourceName, theParamName, join); Predicate pidPredicate = join.get("myTargetResourcePid").in(subQ); @@ -661,7 +571,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateString(String theResourceName, String theParamName, List theList) { - Join join = myResourceTableRoot.join("myParamsString", JoinType.LEFT); + Join join = createOrReuseJoin(JoinEnum.STRING, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -809,7 +719,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateToken(String theResourceName, String theParamName, List theList) { - Join join = myResourceTableRoot.join("myParamsToken", JoinType.LEFT); + Join join = createOrReuseJoin(JoinEnum.TOKEN, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -841,7 +751,7 @@ public class SearchBuilder implements ISearchBuilder { private void addPredicateUri(String theResourceName, String theParamName, List theList) { - Join join = myResourceTableRoot.join("myParamsUri", JoinType.LEFT); + Join join = createOrReuseJoin(JoinEnum.URI, theParamName); if (theList.get(0).getMissing() != null) { addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join); @@ -966,6 +876,42 @@ public class SearchBuilder implements ISearchBuilder { return retVal; } + @SuppressWarnings("unchecked") + private Join createOrReuseJoin(JoinEnum theType, String theSearchParameterName) { + Join join = null; + + switch (theType) { + case DATE: + join = myResourceTableRoot.join("myParamsDate", JoinType.LEFT); + break; + case NUMBER: + join = myResourceTableRoot.join("myParamsNumber", JoinType.LEFT); + break; + case QUANTITY: + join = myResourceTableRoot.join("myParamsQuantity", JoinType.LEFT); + break; + case REFERENCE: + join = myResourceTableRoot.join("myResourceLinks", JoinType.LEFT); + break; + case STRING: + join = myResourceTableRoot.join("myParamsString", JoinType.LEFT); + break; + case URI: + join = myResourceTableRoot.join("myParamsUri", JoinType.LEFT); + break; + case TOKEN: + join = myResourceTableRoot.join("myParamsToken", JoinType.LEFT); + break; + } + + JoinKey key = new JoinKey(theSearchParameterName, theType); + if (!myIndexJoins.containsKey(key)) { + myIndexJoins.put(key, join); + } + + return (Join) join; + } + private Predicate createPredicateDate(IQueryParameterType theParam, String theResourceName, String theParamName, CriteriaBuilder theBuilder, From theFrom) { Predicate p; if (theParam instanceof DateParam) { @@ -1245,10 +1191,10 @@ public class SearchBuilder implements ISearchBuilder { // Use "in" in case of large numbers of codes due to param modifiers final Path systemExpression = theFrom.get("mySystem"); final Path valueExpression = theFrom.get("myValue"); - for (Map.Entry> entry: map.entrySet()) { + for (Map.Entry> entry : map.entrySet()) { Predicate systemPredicate = theBuilder.equal(systemExpression, entry.getKey()); In codePredicate = theBuilder.in(valueExpression); - for (VersionIndependentConcept nextCode: entry.getValue()) { + for (VersionIndependentConcept nextCode : entry.getValue()) { codePredicate.value(nextCode.getCode()); } orPredicates.add(theBuilder.and(systemPredicate, codePredicate)); @@ -1298,8 +1244,6 @@ public class SearchBuilder implements ISearchBuilder { return new QueryIterator(); } - private List myAlsoIncludePids; - private TypedQuery createQuery(SortSpec sort) { CriteriaQuery outerQuery; /* @@ -1344,16 +1288,7 @@ public class SearchBuilder implements ISearchBuilder { } - myResourceTableQuery.distinct(true); myPredicates = new ArrayList(); - if (myParams.getEverythingMode() == null) { - myPredicates.add(myBuilder.equal(myResourceTableRoot.get("myResourceType"), myResourceName)); - } - myPredicates.add(myBuilder.isNull(myResourceTableRoot.get("myDeleted"))); - - DateRangeParam lu = myParams.getLastUpdated(); - List lastUpdatedPredicates = createLastUpdatedPredicates(lu, myBuilder, myResourceTableRoot); - myPredicates.addAll(lastUpdatedPredicates); if (myParams.getEverythingMode() != null) { Join join = myResourceTableRoot.join("myResourceLinks", JoinType.LEFT); @@ -1397,6 +1332,25 @@ public class SearchBuilder implements ISearchBuilder { myPredicates.add(myResourceTableRoot.get("myId").as(Long.class).in(pids)); } + /* + * Add a predicate to make sure we only include non-deleted resources, and only include + * resources of the right type. + * + * If we have any joins to index tables, we get this behaviour already guaranteed so we don't + * need an explicit predicate for it. + */ + if (myIndexJoins.isEmpty()) { + if (myParams.getEverythingMode() == null) { + myPredicates.add(myBuilder.equal(myResourceTableRoot.get("myResourceType"), myResourceName)); + } + myPredicates.add(myBuilder.isNull(myResourceTableRoot.get("myDeleted"))); + } + + // Last updated + DateRangeParam lu = myParams.getLastUpdated(); + List lastUpdatedPredicates = createLastUpdatedPredicates(lu, myBuilder, myResourceTableRoot); + myPredicates.addAll(lastUpdatedPredicates); + myResourceTableQuery.where(myBuilder.and(SearchBuilder.toArray(myPredicates))); /* @@ -1450,47 +1404,65 @@ public class SearchBuilder implements ISearchBuilder { String joinAttrName; String[] sortAttrName; + JoinEnum joinType; switch (param.getParamType()) { case STRING: joinAttrName = "myParamsString"; sortAttrName = new String[] { "myValueExact" }; + joinType = JoinEnum.STRING; break; case DATE: joinAttrName = "myParamsDate"; sortAttrName = new String[] { "myValueLow" }; + joinType = JoinEnum.DATE; break; case REFERENCE: joinAttrName = "myResourceLinks"; sortAttrName = new String[] { "myTargetResourcePid" }; + joinType = JoinEnum.REFERENCE; break; case TOKEN: joinAttrName = "myParamsToken"; sortAttrName = new String[] { "mySystem", "myValue" }; + joinType = JoinEnum.TOKEN; break; case NUMBER: joinAttrName = "myParamsNumber"; sortAttrName = new String[] { "myValue" }; + joinType = JoinEnum.NUMBER; break; case URI: joinAttrName = "myParamsUri"; sortAttrName = new String[] { "myUri" }; + joinType = JoinEnum.URI; break; case QUANTITY: joinAttrName = "myParamsQuantity"; sortAttrName = new String[] { "myValue" }; + joinType = JoinEnum.QUANTITY; break; default: throw new InvalidRequestException("This server does not support _sort specifications of type " + param.getParamType() + " - Can't serve _sort=" + theSort.getParamName()); } - From join = theFrom.join(joinAttrName, JoinType.LEFT); + /* + * If we've already got a join for the specific parameter we're + * sorting on, we'll also sort with it. Otherwise we need a new join. + */ + JoinKey key = new JoinKey(theSort.getParamName(), joinType); + Join join = myIndexJoins.get(key); + if (join == null) { + join = theFrom.join(joinAttrName, JoinType.LEFT); - if (param.getParamType() == RestSearchParameterTypeEnum.REFERENCE) { - thePredicates.add(join.get("mySourcePath").as(String.class).in(param.getPathsSplit())); + if (param.getParamType() == RestSearchParameterTypeEnum.REFERENCE) { + thePredicates.add(join.get("mySourcePath").as(String.class).in(param.getPathsSplit())); + } else { + Predicate joinParam1 = theBuilder.equal(join.get("myParamName"), theSort.getParamName()); + thePredicates.add(joinParam1); + } } else { - Predicate joinParam1 = theBuilder.equal(join.get("myParamName"), theSort.getParamName()); - thePredicates.add(joinParam1); + ourLog.info("Reusing join for {}", theSort.getParamName()); } for (String next : sortAttrName) { @@ -1536,41 +1508,6 @@ public class SearchBuilder implements ISearchBuilder { return retVal; } - @Override - public void loadResourcesByPid(Collection theIncludePids, List theResourceListToPopulate, Set theRevIncludedPids, boolean theForHistoryOperation, - EntityManager entityManager, FhirContext context, IDao theDao) { - if (theIncludePids.isEmpty()) { - ourLog.info("The include pids are empty"); - //return; - } - - // Dupes will cause a crash later anyhow, but this is expensive so only do it - // when running asserts - assert new HashSet(theIncludePids).size() == theIncludePids.size() : "PID list contains duplicates: " + theIncludePids; - - Map position = new HashMap(); - for (Long next : theIncludePids) { - position.put(next, theResourceListToPopulate.size()); - theResourceListToPopulate.add(null); - } - - /* - * As always, Oracle can't handle things that other databases don't mind.. In this - * case it doesn't like more than ~1000 IDs in a single load, so we break this up - * if it's lots of IDs. I suppose maybe we should be doing this as a join anyhow - * but this should work too. Sigh. - */ - int maxLoad = 800; - List pids = new ArrayList(theIncludePids); - for (int i = 0; i < pids.size(); i += maxLoad) { - int to = i + maxLoad; - to = Math.min(to, pids.size()); - List pidsSubList = pids.subList(i, to); - doLoadPids(theResourceListToPopulate, theRevIncludedPids, theForHistoryOperation, entityManager, context, theDao, position, pidsSubList); - } - - } - private void doLoadPids(List theResourceListToPopulate, Set theRevIncludedPids, boolean theForHistoryOperation, EntityManager entityManager, FhirContext context, IDao theDao, Map position, Collection pids) { CriteriaBuilder builder = entityManager.getCriteriaBuilder(); @@ -1606,6 +1543,41 @@ public class SearchBuilder implements ISearchBuilder { } } + @Override + public void loadResourcesByPid(Collection theIncludePids, List theResourceListToPopulate, Set theRevIncludedPids, boolean theForHistoryOperation, + EntityManager entityManager, FhirContext context, IDao theDao) { + if (theIncludePids.isEmpty()) { + ourLog.info("The include pids are empty"); + // return; + } + + // Dupes will cause a crash later anyhow, but this is expensive so only do it + // when running asserts + assert new HashSet(theIncludePids).size() == theIncludePids.size() : "PID list contains duplicates: " + theIncludePids; + + Map position = new HashMap(); + for (Long next : theIncludePids) { + position.put(next, theResourceListToPopulate.size()); + theResourceListToPopulate.add(null); + } + + /* + * As always, Oracle can't handle things that other databases don't mind.. In this + * case it doesn't like more than ~1000 IDs in a single load, so we break this up + * if it's lots of IDs. I suppose maybe we should be doing this as a join anyhow + * but this should work too. Sigh. + */ + int maxLoad = 800; + List pids = new ArrayList(theIncludePids); + for (int i = 0; i < pids.size(); i += maxLoad) { + int to = i + maxLoad; + to = Math.min(to, pids.size()); + List pidsSubList = pids.subList(i, to); + doLoadPids(theResourceListToPopulate, theRevIncludedPids, theForHistoryOperation, entityManager, context, theDao, position, pidsSubList); + } + + } + /** * THIS SHOULD RETURN HASHSET and not jsut Set because we add to it later (so it can't be Collections.emptySet()) * @@ -1844,7 +1816,7 @@ public class SearchBuilder implements ISearchBuilder { } @Override - public void setType(Class theResourceType, String theResourceName) { + public void setType(Class theResourceType, String theResourceName) { myResourceType = theResourceType; myResourceName = theResourceName; } @@ -1963,13 +1935,46 @@ public class SearchBuilder implements ISearchBuilder { return thePredicates.toArray(new Predicate[thePredicates.size()]); } + private enum JoinEnum { + DATE, NUMBER, QUANTITY, REFERENCE, STRING, TOKEN, URI + + } + + private static class JoinKey { + private final JoinEnum myJoinType; + private final String myParamName; + + public JoinKey(String theParamName, JoinEnum theJoinType) { + super(); + myParamName = theParamName; + myJoinType = theJoinType; + } + + @Override + public boolean equals(Object theObj) { + JoinKey obj = (JoinKey) theObj; + return new EqualsBuilder() + .append(myParamName, obj.myParamName) + .append(myJoinType, obj.myJoinType) + .isEquals(); + } + + @Override + public int hashCode() { + return new HashCodeBuilder() + .append(myParamName) + .append(myJoinType) + .toHashCode(); + } + } + private final class QueryIterator implements Iterator { + private boolean myFirst = true; private Long myNext; private final Set myPidSet = new HashSet(); + private Iterator myPreResultsIterator; private Iterator myResultsIterator; private SortSpec mySort; - private Iterator myPreResultsIterator; - private boolean myFirst = true; private StopWatch myStopwatch = null; private QueryIterator() { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java index ee21079fe49..4f4cad1946c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/data/IResourceTableDao.java @@ -1,5 +1,7 @@ package ca.uhn.fhir.jpa.dao.data; +import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Slice; /* * #%L * HAPI FHIR JPA Server @@ -19,10 +21,7 @@ package ca.uhn.fhir.jpa.dao.data; * limitations under the License. * #L% */ - -import org.springframework.data.jpa.repository.JpaRepository; -import org.springframework.data.jpa.repository.Modifying; -import org.springframework.data.jpa.repository.Query; +import org.springframework.data.jpa.repository.*; import org.springframework.data.repository.query.Param; import ca.uhn.fhir.jpa.entity.ResourceTable; @@ -33,4 +32,7 @@ public interface IResourceTableDao extends JpaRepository { @Query("UPDATE ResourceTable r SET r.myIndexStatus = null WHERE r.myResourceType = :restype") int markResourcesOfTypeAsRequiringReindexing(@Param("restype") String theResourceType); + @Query("SELECT t.myId FROM ResourceTable t WHERE t.myIndexStatus IS NULL") + Slice findUnindexed(Pageable thePageRequest); + } diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java index 69f1e5f0920..578edef274d 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java @@ -1,6 +1,7 @@ package ca.uhn.fhir.jpa.config; import java.util.Properties; +import java.util.concurrent.TimeUnit; import javax.persistence.EntityManagerFactory; import javax.sql.DataSource; @@ -17,6 +18,8 @@ import org.springframework.transaction.annotation.EnableTransactionManagement; import ca.uhn.fhir.jpa.dao.DaoConfig; import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; import ca.uhn.fhir.validation.ResultSeverityEnum; +import net.ttddyy.dsproxy.listener.logging.SLF4JLogLevel; +import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; @Configuration @EnableTransactionManagement() @@ -34,7 +37,15 @@ public class TestDstu3Config extends BaseJavaConfigDstu3 { retVal.setUrl("jdbc:derby:memory:myUnitTestDB;create=true"); retVal.setUsername(""); retVal.setPassword(""); - return retVal; + + DataSource dataSource = ProxyDataSourceBuilder + .create(retVal) + .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") + .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) + .countQuery() + .build(); + + return dataSource; } @Bean() diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index 69f46d46fcb..152f6479d89 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -1827,6 +1827,32 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { } + @Test + public void testSearchStringParamDoesntMatchWrongType() throws Exception { + IIdType pid1; + IIdType pid2; + { + Patient patient = new Patient(); + patient.addName().setFamily("HELLO"); + pid1 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + } + { + Practitioner patient = new Practitioner(); + patient.addName().setFamily("HELLO"); + pid2 = myPractitionerDao.create(patient, mySrd).getId().toUnqualifiedVersionless(); + } + + SearchParameterMap params; + List patients; + + params = new SearchParameterMap(); + params.add(Patient.SP_FAMILY, new StringParam("HELLO")); + patients = toUnqualifiedVersionlessIds(myPatientDao.search(params)); + assertThat(patients, containsInAnyOrder(pid1)); + assertThat(patients, not(containsInAnyOrder(pid2))); + } + + @Test public void testSearchStringParam() throws Exception { IIdType pid1; @@ -3251,11 +3277,26 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { SearchParameterMap map; List ids; + // No search param map = new SearchParameterMap(); map.setSort(new SortSpec("family", SortOrderEnum.ASC).setChain(new SortSpec("given", SortOrderEnum.ASC))); ids = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); assertThat(ids, contains("Patient/AA", "Patient/AB", "Patient/BA", "Patient/BB")); + // Same SP as sort + map = new SearchParameterMap(); + map.add(Patient.SP_ACTIVE, new TokenParam(null, "true")); + map.setSort(new SortSpec("family", SortOrderEnum.ASC).setChain(new SortSpec("given", SortOrderEnum.ASC))); + ids = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(ids, contains("Patient/AA", "Patient/AB", "Patient/BA", "Patient/BB")); + + // Different SP from sort + map = new SearchParameterMap(); + map.add(Patient.SP_GENDER, new TokenParam(null, "male")); + map.setSort(new SortSpec("family", SortOrderEnum.ASC).setChain(new SortSpec("given", SortOrderEnum.ASC))); + ids = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); + assertThat(ids, contains("Patient/AA", "Patient/AB", "Patient/BA", "Patient/BB")); + map = new SearchParameterMap(); map.setSort(new SortSpec("gender").setChain(new SortSpec("family", SortOrderEnum.ASC).setChain(new SortSpec("given", SortOrderEnum.ASC)))); ids = toUnqualifiedVersionlessIdValues(myPatientDao.search(map)); diff --git a/pom.xml b/pom.xml index 16dc608f397..9d8cb8cd28d 100644 --- a/pom.xml +++ b/pom.xml @@ -538,6 +538,11 @@ Saxon-HE 9.5.1-5 + + net.ttddyy + datasource-proxy + 1.4.1 + org.apache.commons commons-dbcp2 diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7dc713b157d..93ce40e6fa1 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -24,6 +24,10 @@ looked like before the update. This change was made to support the change above, but seems like a useful feature all around. + + Optimize queries in JPA server remove a few redundant select columns when performing + searches. This provides a slight speed increase in some cases. + From d40c5fa5e3ad3b93ed2552209371603954a9eda9 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 16 Jun 2017 09:41:45 -0400 Subject: [PATCH 2/8] Add DAO setting to specify maximum query size --- .../fhir/jpa/dao/BaseHapiFhirSystemDao.java | 13 +++- .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 73 ++++++++++++++----- .../FhirResourceDaoDstu3SearchNoFtTest.java | 20 ++++- src/changes/changes.xml | 7 ++ 4 files changed, 88 insertions(+), 25 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java index 5ed0a47b7f0..3e0ae8af35b 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java @@ -104,7 +104,7 @@ public abstract class BaseHapiFhirSystemDao extends BaseHapiFhirDao q = myEntityManager.createQuery("SELECT t.myId FROM ResourceTable t WHERE t.myIndexStatus IS NULL", Long.class); - ourLog.info("Beginning indexing query with maximum {}", maxResult); + ourLog.debug("Beginning indexing query with maximum {}", maxResult); q.setMaxResults(maxResult); Collection resources = q.getResultList(); @@ -145,9 +145,14 @@ public abstract class BaseHapiFhirSystemDao extends BaseHapiFhirDao 0 ? (delay / count) : 0; - ourLog.info("Indexed {} resources in {}ms - Avg {}ms / resource", new Object[] { count, delay, avg }); - + long avg; + if (count > 0) { + avg = (delay / count); + ourLog.info("Indexed {} resources in {}ms - Avg {}ms / resource", new Object[] { count, delay, avg }); + } else { + ourLog.debug("Indexed 0 resources in {}ms", delay); + } + return count; } }); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java index 067a07e32a1..56aee54fbd7 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java @@ -64,21 +64,21 @@ public class DaoConfig { */ public static final Long DEFAULT_REUSE_CACHED_SEARCH_RESULTS_FOR_MILLIS = DateUtils.MILLIS_PER_MINUTE; - // *** - // update setter javadoc if default changes - // *** + /** + * update setter javadoc if default changes + */ private boolean myAllowExternalReferences = false; - // *** - // update setter javadoc if default changes - // *** + /** + * update setter javadoc if default changes + */ private boolean myAllowInlineMatchUrlReferences = true; private boolean myAllowMultipleDelete; private boolean myDefaultSearchParamsCanBeOverridden = false; - // *** - // update setter javadoc if default changes - // *** + /** + * update setter javadoc if default changes + */ private int myDeferIndexingForCodesystemsOfSize = 2000; private boolean myDeleteStaleSearches = true; @@ -87,27 +87,34 @@ public class DaoConfig { private boolean myEnforceReferentialIntegrityOnWrite = true; - // *** - // update setter javadoc if default changes - // *** + /** + * update setter javadoc if default changes + */ private long myExpireSearchResultsAfterMillis = DateUtils.MILLIS_PER_HOUR; - private int myHardTagListLimit = 1000; + /** + * update setter javadoc if default changes + */ + private Integer myFetchSizeDefaultMaximum = null; + private int myHardTagListLimit = 1000; private int myIncludeLimit = 2000; - // *** - // update setter javadoc if default changes - // *** + /** + * update setter javadoc if default changes + */ private boolean myIndexContainedResources = true; private List myInterceptors; - // *** - // update setter javadoc if default changes - // *** + /** + * update setter javadoc if default changes + */ private int myMaximumExpansionSize = 5000; private int myMaximumSearchResultCountInTransaction = DEFAULT_MAXIMUM_SEARCH_RESULT_COUNT_IN_TRANSACTION; private ResourceEncodingEnum myResourceEncoding = ResourceEncodingEnum.JSONC; private Long myReuseCachedSearchResultsForMillis = DEFAULT_REUSE_CACHED_SEARCH_RESULTS_FOR_MILLIS; private boolean mySchedulingDisabled; private boolean mySubscriptionEnabled; + /** + * update setter javadoc if default changes + */ private long mySubscriptionPollDelay = 1000; private Long mySubscriptionPurgeInactiveAfterMillis; private boolean mySuppressUpdatesWithNoChange = true; @@ -160,6 +167,20 @@ public class DaoConfig { return myExpireSearchResultsAfterMillis; } + /** + * Gets the default maximum number of results to load in a query. + *

+ * For example, if the database has a million Patient resources in it, and + * the client requests GET /Patient, if this value is set + * to a non-null value (default is null) only this number + * of results will be fetched. Setting this value appropriately + * can be useful to improve performance in some situations. + *

+ */ + public Integer getFetchSizeDefaultMaximum() { + return myFetchSizeDefaultMaximum; + } + /** * Gets the maximum number of results to return in a GetTags query (DSTU1 only) */ @@ -549,6 +570,20 @@ public class DaoConfig { myExpireSearchResultsAfterMillis = theExpireSearchResultsAfterMillis; } + /** + * Gets the default maximum number of results to load in a query. + *

+ * For example, if the database has a million Patient resources in it, and + * the client requests GET /Patient, if this value is set + * to a non-null value (default is null) only this number + * of results will be fetched. Setting this value appropriately + * can be useful to improve performance in some situations. + *

+ */ + public void setFetchSizeDefaultMaximum(Integer theFetchSizeDefaultMaximum) { + myFetchSizeDefaultMaximum = theFetchSizeDefaultMaximum; + } + /** * Do not call this method, it exists only for legacy reasons. It * will be removed in a future version. Configure the page size on your diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index 152f6479d89..e9cb2b3080c 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -82,8 +82,7 @@ import org.springframework.transaction.TransactionStatus; import org.springframework.transaction.support.TransactionCallback; import org.springframework.transaction.support.TransactionTemplate; -import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao; -import ca.uhn.fhir.jpa.dao.SearchParameterMap; +import ca.uhn.fhir.jpa.dao.*; import ca.uhn.fhir.jpa.dao.SearchParameterMap.EverythingModeEnum; import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamDate; import ca.uhn.fhir.jpa.entity.ResourceIndexedSearchParamNumber; @@ -130,6 +129,7 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { @Before public void beforeDisableResultReuse() { myDaoConfig.setReuseCachedSearchResultsForMillis(null); + myDaoConfig.setFetchSizeDefaultMaximum(new DaoConfig().getFetchSizeDefaultMaximum()); } /** @@ -1852,6 +1852,22 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { assertThat(patients, not(containsInAnyOrder(pid2))); } + @Test + public void testSearchWithFetchSizeDefaultMaximum() { + myDaoConfig.setFetchSizeDefaultMaximum(5); + + for (int i = 0; i < 10; i++) { + Patient p = new Patient(); + p.addName().setFamily("PT" + i); + myPatientDao.create(p); + } + + SearchParameterMap map = new SearchParameterMap(); + map.setLoadSynchronous(true); + IBundleProvider values = myPatientDao.search(map); + assertEquals(5, values.size().intValue()); + assertEquals(5, values.getResources(0, 1000).size()); + } @Test public void testSearchStringParam() throws Exception { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 93ce40e6fa1..8c12874f002 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -28,6 +28,13 @@ Optimize queries in JPA server remove a few redundant select columns when performing searches. This provides a slight speed increase in some cases. + + Add configuration to JPA server DaoConfig that allows a maximum + number of search results to be specified. Queries will never return + more than this number, which can be good for avoiding accidental + performance problems in situations where lare queries should not be + needed + From 507cac31271aeb8019d31758d8716c5dee932a46 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 16 Jun 2017 09:52:12 -0400 Subject: [PATCH 3/8] Respect prefer header on transaction processing --- .../SimpleRequestHeaderInterceptor.java | 90 +++++++++++++++++++ .../jpa/dao/dstu3/FhirSystemDaoDstu3.java | 29 ++++-- .../FhirResourceDaoDstu3SearchNoFtTest.java | 17 ++++ .../dstu3/ResourceProviderDstu3Test.java | 47 ++++++++++ .../dstu3/SystemProviderDstu3Test.java | 47 ++++++++++ .../rest/client/GenericClientDstu3Test.java | 34 +++++++ .../rest/client/SearchClientDstu3Test.java | 1 - src/changes/changes.xml | 5 ++ 8 files changed, 261 insertions(+), 9 deletions(-) create mode 100644 hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/interceptor/SimpleRequestHeaderInterceptor.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/interceptor/SimpleRequestHeaderInterceptor.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/interceptor/SimpleRequestHeaderInterceptor.java new file mode 100644 index 00000000000..63c7a25f90f --- /dev/null +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/interceptor/SimpleRequestHeaderInterceptor.java @@ -0,0 +1,90 @@ +package ca.uhn.fhir.rest.client.interceptor; + +import static org.apache.commons.lang3.StringUtils.isNotBlank; + +/* + * #%L + * HAPI FHIR - Core Library + * %% + * Copyright (C) 2014 - 2017 University Health Network + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + +import java.io.IOException; +import java.io.UnsupportedEncodingException; + +import org.apache.commons.codec.binary.Base64; +import org.apache.commons.lang3.StringUtils; + +import ca.uhn.fhir.rest.client.IClientInterceptor; +import ca.uhn.fhir.rest.client.api.IHttpRequest; +import ca.uhn.fhir.rest.client.api.IHttpResponse; +import ca.uhn.fhir.rest.server.Constants; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; + +/** + * This interceptor adds an arbitrary header to requests made by this client. Both the + * header name and the header value are specified by the calling code. + */ +public class SimpleRequestHeaderInterceptor implements IClientInterceptor { + + private String myHeaderName; + private String myHeaderValue; + + /** + * Constructor + */ + public SimpleRequestHeaderInterceptor() { + this(null, null); + } + + /** + * Constructor + */ + public SimpleRequestHeaderInterceptor(String theHeaderName, String theHeaderValue) { + super(); + myHeaderName = theHeaderName; + myHeaderValue = theHeaderValue; + } + + public String getHeaderName() { + return myHeaderName; + } + + public String getHeaderValue() { + return myHeaderValue; + } + + @Override + public void interceptRequest(IHttpRequest theRequest) { + if (isNotBlank(getHeaderName())) { + theRequest.addHeader(getHeaderName(), getHeaderValue()); + } + } + + @Override + public void interceptResponse(IHttpResponse theResponse) throws IOException { + // nothing + } + + public void setHeaderName(String theHeaderName) { + myHeaderName = theHeaderName; + } + + public void setHeaderValue(String theHeaderValue) { + myHeaderValue = theHeaderValue; + } + +} diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java index 109a433a52b..6809fa97daf 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java @@ -39,6 +39,7 @@ import java.util.Map.Entry; import java.util.Set; import javax.persistence.TypedQuery; +import javax.servlet.http.HttpServletRequest; import org.apache.http.NameValuePair; import org.hl7.fhir.dstu3.model.Bundle; @@ -79,6 +80,7 @@ import ca.uhn.fhir.jpa.util.DeleteConflict; import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.rest.api.PreferReturnEnum; import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.method.BaseMethodBinding; @@ -282,7 +284,7 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { super.clearRequestAsProcessingSubRequest(theRequestDetails); } } - + @SuppressWarnings("unchecked") private Bundle doTransaction(ServletRequestDetails theRequestDetails, Bundle theRequest, String theActionName) { BundleType transactionType = theRequest.getTypeElement().getValue(); @@ -408,7 +410,7 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { String matchUrl = nextReqEntry.getRequest().getIfNoneExist(); outcome = resourceDao.create(res, matchUrl, false, theRequestDetails); if (nextResourceId != null) { - handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res); + handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequestDetails); } entriesToProcess.put(nextRespEntry, outcome.getEntity()); if (outcome.getCreated() == false) { @@ -445,12 +447,12 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { if (allDeleted.isEmpty()) { status = Constants.STATUS_HTTP_204_NO_CONTENT; } - + nextRespEntry.getResponse().setOutcome((Resource) deleteOutcome.getOperationOutcome()); } nextRespEntry.getResponse().setStatus(toStatusString(status)); - + break; } case PUT: { @@ -474,7 +476,7 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { } } - handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res); + handleTransactionCreateOrUpdateOutcome(idSubstitutions, idToPersistedOutcome, nextResourceId, outcome, nextRespEntry, resourceType, res, theRequestDetails); entriesToProcess.put(nextRespEntry, outcome.getEntity()); break; } @@ -646,10 +648,8 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { return response; } - - private static void handleTransactionCreateOrUpdateOutcome(Map idSubstitutions, Map idToPersistedOutcome, IdType nextResourceId, DaoMethodOutcome outcome, - BundleEntryComponent newEntry, String theResourceType, IBaseResource theRes) { + BundleEntryComponent newEntry, String theResourceType, IBaseResource theRes, ServletRequestDetails theRequestDetails) { IdType newId = (IdType) outcome.getId().toUnqualifiedVersionless(); IdType resourceId = isPlaceholder(nextResourceId) ? nextResourceId : nextResourceId.toUnqualifiedVersionless(); if (newId.equals(resourceId) == false) { @@ -668,6 +668,19 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { newEntry.getResponse().setStatus(toStatusString(Constants.STATUS_HTTP_200_OK)); } newEntry.getResponse().setLastModified(((Resource) theRes).getMeta().getLastUpdated()); + + if (theRequestDetails != null) { + if (outcome.getResource() != null) { + String prefer = theRequestDetails.getHeader(Constants.HEADER_PREFER); + PreferReturnEnum preferReturn = RestfulServerUtils.parsePreferHeader(prefer); + if (preferReturn != null) { + if (preferReturn == PreferReturnEnum.REPRESENTATION) { + newEntry.setResource((Resource) outcome.getResource()); + } + } + } + } + } private static boolean isPlaceholder(IdType theId) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java index 69f46d46fcb..1ea1fb48862 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirResourceDaoDstu3SearchNoFtTest.java @@ -1181,6 +1181,23 @@ public class FhirResourceDaoDstu3SearchNoFtTest extends BaseJpaDstu3Test { assertThat(patients, (hasItems(id1a, id1b))); assertThat(patients, not(hasItems(id2))); } + + + { + SearchParameterMap params = new SearchParameterMap(); + params.setLastUpdated(new DateRangeParam(new DateParam(ParamPrefixEnum.GREATERTHAN_OR_EQUALS, beforeR2))); + List patients = toUnqualifiedVersionlessIds(myPatientDao.search(params)); + assertThat(patients, not(hasItems(id1a, id1b))); + assertThat(patients, (hasItems(id2))); + } + { + SearchParameterMap params = new SearchParameterMap(); + params.setLastUpdated(new DateRangeParam(new DateParam(ParamPrefixEnum.LESSTHAN_OR_EQUALS, beforeR2))); + List patients = toUnqualifiedVersionlessIds(myPatientDao.search(params)); + assertThat(patients, (hasItems(id1a, id1b))); + assertThat(patients, not(hasItems(id2))); + } + } @SuppressWarnings("deprecation") diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 7d048e7f8c2..eee52e83a00 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -1448,6 +1448,53 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info(ids.toString()); } + @Test + public void testSearchByLastUpdated() throws Exception { + String methodName = "testSearchByLastUpdated"; + + Patient p = new Patient(); + p.addName().setFamily(methodName+"1"); + IIdType pid1 = ourClient.create().resource(p).execute().getId().toUnqualifiedVersionless(); + + Thread.sleep(10); + long time1 = System.currentTimeMillis(); + Thread.sleep(10); + + Patient p2 = new Patient(); + p2.addName().setFamily(methodName+"2"); + IIdType pid2 = ourClient.create().resource(p2).execute().getId().toUnqualifiedVersionless(); + + HttpGet get = new HttpGet(ourServerBase + "/Patient?_lastUpdated=lt" + new InstantType(new Date(time1)).getValueAsString()); + CloseableHttpResponse response = ourHttpClient.execute(get); + try { + assertEquals(200, response.getStatusLine().getStatusCode()); + String output = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + IOUtils.closeQuietly(response.getEntity().getContent()); + ourLog.info(output); + List ids = toUnqualifiedVersionlessIds(myFhirCtx.newXmlParser().parseResource(Bundle.class, output)); + ourLog.info(ids.toString()); + assertThat(ids, containsInAnyOrder(pid1)); + } finally { + response.close(); + } + + get = new HttpGet(ourServerBase + "/Patient?_lastUpdated=gt" + new InstantType(new Date(time1)).getValueAsString()); + response = ourHttpClient.execute(get); + try { + assertEquals(200, response.getStatusLine().getStatusCode()); + String output = IOUtils.toString(response.getEntity().getContent(), StandardCharsets.UTF_8); + IOUtils.closeQuietly(response.getEntity().getContent()); + ourLog.info(output); + List ids = toUnqualifiedVersionlessIds(myFhirCtx.newXmlParser().parseResource(Bundle.class, output)); + ourLog.info(ids.toString()); + assertThat(ids, containsInAnyOrder(pid2)); + } finally { + response.close(); + } + + } + + @Test public void testEverythingPatientType() throws Exception { String methodName = "testEverythingPatientType"; diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java index e4c70769d94..af577fef5db 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/SystemProviderDstu3Test.java @@ -60,6 +60,7 @@ import ca.uhn.fhir.jpa.rp.dstu3.OrganizationResourceProvider; import ca.uhn.fhir.jpa.rp.dstu3.PatientResourceProvider; import ca.uhn.fhir.jpa.testutil.RandomServerPortProvider; import ca.uhn.fhir.rest.client.IGenericClient; +import ca.uhn.fhir.rest.client.interceptor.SimpleRequestHeaderInterceptor; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.rest.server.EncodingEnum; import ca.uhn.fhir.rest.server.FifoMemoryPagingProvider; @@ -80,6 +81,7 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SystemProviderDstu3Test.class); private static Server ourServer; private static String ourServerBase; + private SimpleRequestHeaderInterceptor mySimpleHeaderInterceptor; @Test public void testTransactionWithInlineConditionalUrl() throws Exception { @@ -237,10 +239,17 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { myRestServer.setDefaultResponseEncoding(EncodingEnum.XML); } + @Before + public void before() { + mySimpleHeaderInterceptor = new SimpleRequestHeaderInterceptor(); + ourClient.registerInterceptor(mySimpleHeaderInterceptor); + } + @SuppressWarnings("deprecation") @After public void after() { myRestServer.setUseBrowserFriendlyContentTypes(true); + ourClient.unregisterInterceptor(mySimpleHeaderInterceptor); } @SuppressWarnings("deprecation") @@ -632,6 +641,44 @@ public class SystemProviderDstu3Test extends BaseJpaDstu3Test { assertEquals(0, respSub.getEntry().size()); } + @Test + public void testTransactionCreateWithPreferHeader() throws Exception { + + Patient p = new Patient(); + p.setActive(true); + + Bundle req; + Bundle resp; + + // No prefer header + req = new Bundle(); + req.setType(BundleType.TRANSACTION); + req.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient"); + resp = ourClient.transaction().withBundle(req).execute(); + assertEquals(null, resp.getEntry().get(0).getResource()); + assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus()); + + // Prefer return=minimal + mySimpleHeaderInterceptor.setHeaderName(Constants.HEADER_PREFER); + mySimpleHeaderInterceptor.setHeaderValue(Constants.HEADER_PREFER_RETURN + "=" + Constants.HEADER_PREFER_RETURN_MINIMAL); + req = new Bundle(); + req.setType(BundleType.TRANSACTION); + req.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient"); + resp = ourClient.transaction().withBundle(req).execute(); + assertEquals(null, resp.getEntry().get(0).getResource()); + assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus()); + + // Prefer return=representation + mySimpleHeaderInterceptor.setHeaderName(Constants.HEADER_PREFER); + mySimpleHeaderInterceptor.setHeaderValue(Constants.HEADER_PREFER_RETURN + "=" + Constants.HEADER_PREFER_RETURN_REPRESENTATION); + req = new Bundle(); + req.setType(BundleType.TRANSACTION); + req.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.POST).setUrl("Patient"); + resp = ourClient.transaction().withBundle(req).execute(); + assertEquals(Patient.class, resp.getEntry().get(0).getResource().getClass()); + assertEquals("201 Created", resp.getEntry().get(0).getResponse().getStatus()); + } + @AfterClass public static void afterClassClearContext() throws Exception { ourServer.stop(); diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java index 16674c77d0c..5f7e5b8ff9e 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java @@ -27,6 +27,7 @@ import org.apache.http.ProtocolVersion; import org.apache.http.client.ClientProtocolException; import org.apache.http.client.HttpClient; import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; +import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.entity.ContentType; @@ -60,6 +61,7 @@ import ca.uhn.fhir.parser.CustomTypeDstu3Test.MyCustomPatient; import ca.uhn.fhir.parser.IParser; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.PreferReturnEnum; +import ca.uhn.fhir.rest.client.SearchClientDstu3Test.ILocationClient; import ca.uhn.fhir.rest.client.exceptions.FhirClientConnectionException; import ca.uhn.fhir.rest.client.exceptions.NonFhirResponseException; import ca.uhn.fhir.rest.client.interceptor.CookieInterceptor; @@ -177,6 +179,8 @@ public class GenericClientDstu3Test { OperationOutcome oo = (OperationOutcome) outcome.getOperationOutcome(); assertThat(oo.getText().getDivAsString(), containsString("OK!")); } + + @Test public void testPatchJsonByIdType() throws Exception { @@ -1191,6 +1195,36 @@ public class GenericClientDstu3Test { // assertEquals("FAM", resp.getNameFirstRep().getFamilyAsSingleString()); } + @Test + public void testSearchWithNullParameters() throws Exception { + final String msg = "{\"resourceType\":\"Bundle\",\"id\":null,\"base\":\"http://localhost:57931/fhir/contextDev\",\"total\":1,\"link\":[{\"relation\":\"self\",\"url\":\"http://localhost:57931/fhir/contextDev/Patient?identifier=urn%3AMultiFhirVersionTest%7CtestSubmitPatient01&_format=json\"}],\"entry\":[{\"resource\":{\"resourceType\":\"Patient\",\"id\":\"1\",\"meta\":{\"versionId\":\"1\",\"lastUpdated\":\"2014-12-20T18:41:29.706-05:00\"},\"identifier\":[{\"system\":\"urn:MultiFhirVersionTest\",\"value\":\"testSubmitPatient01\"}]}}]}"; + + ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); + when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse); + when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); + when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_JSON + "; charset=UTF-8")); + when(myHttpResponse.getEntity().getContent()).then(new Answer() { + @Override + public InputStream answer(InvocationOnMock theInvocation) throws Throwable { + return new ReaderInputStream(new StringReader(msg), Charset.forName("UTF-8")); + } + }); + + IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir"); + int idx = 0; + + DateTimeDt now = DateTimeDt.withCurrentTime(); + String dateString = now.getValueAsString().substring(0, 10); + + client.search() + .forResource("Patient") + .where(Patient.NAME.matches().value((String)null)) + .returnBundle(Bundle.class) + .execute(); + assertEquals("http://example.com/fhir/Patient", capt.getAllValues().get(idx).getURI().toString()); + idx++; + } + @Test public void testSearchByDate() throws Exception { final String msg = "{\"resourceType\":\"Bundle\",\"id\":null,\"base\":\"http://localhost:57931/fhir/contextDev\",\"total\":1,\"link\":[{\"relation\":\"self\",\"url\":\"http://localhost:57931/fhir/contextDev/Patient?identifier=urn%3AMultiFhirVersionTest%7CtestSubmitPatient01&_format=json\"}],\"entry\":[{\"resource\":{\"resourceType\":\"Patient\",\"id\":\"1\",\"meta\":{\"versionId\":\"1\",\"lastUpdated\":\"2014-12-20T18:41:29.706-05:00\"},\"identifier\":[{\"system\":\"urn:MultiFhirVersionTest\",\"value\":\"testSubmitPatient01\"}]}}]}"; diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/SearchClientDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/SearchClientDstu3Test.java index 51a959d7c4f..ddfa5e2849f 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/SearchClientDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/SearchClientDstu3Test.java @@ -43,7 +43,6 @@ import ca.uhn.fhir.rest.annotation.Sort; import ca.uhn.fhir.rest.api.SortOrderEnum; import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.client.api.IRestfulClient; -import ca.uhn.fhir.rest.param.DateParam; import ca.uhn.fhir.rest.param.StringParam; import ca.uhn.fhir.rest.server.Constants; import ca.uhn.fhir.util.TestUtil; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index edf36e28bf2..9d7e9731acb 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -28,6 +28,11 @@ Fix HTTP 500 error in JPA server if a numeric search parameter was supplied with no value, e.g. GET /Observation?value-quantity=]]> + + JPA server transaction processing now honours the Prefer header and includes + created and updated resource bodies in the response bundle if it is set + appropriately. + From 1047c63f3cce6d109261c19cd9abbbde97e1426e Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 16 Jun 2017 10:35:39 -0400 Subject: [PATCH 4/8] Add missing commit for JPA maximum fetch size --- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) 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 a723bd9682a..e2d6223fa51 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 @@ -43,16 +43,7 @@ import org.hl7.fhir.instance.model.api.*; import com.google.common.collect.*; -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.FhirVersionEnum; -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.context.*; import ca.uhn.fhir.jpa.dao.data.IForcedIdDao; import ca.uhn.fhir.jpa.dao.data.IResourceIndexedSearchParamUriDao; import ca.uhn.fhir.jpa.entity.*; @@ -1253,7 +1244,7 @@ public class SearchBuilder implements ISearchBuilder { return new QueryIterator(); } - private TypedQuery createQuery(SortSpec sort) { + private TypedQuery createQuery(SortSpec sort, Integer theMaximumResults) { CriteriaQuery outerQuery; /* * Sort @@ -1366,6 +1357,11 @@ public class SearchBuilder implements ISearchBuilder { * Now perform the search */ final TypedQuery query = myEntityManager.createQuery(outerQuery); + + if (theMaximumResults != null) { + query.setMaxResults(theMaximumResults); + } + return query; } @@ -2031,7 +2027,9 @@ public class SearchBuilder implements ISearchBuilder { // If we don't have a query yet, create one if (myResultsIterator == null) { - final TypedQuery query = createQuery(mySort); + Integer maximumResults = myCallingDao.getConfig().getFetchSizeDefaultMaximum(); + + final TypedQuery query = createQuery(mySort, maximumResults); myResultsIterator = query.getResultList().iterator(); // If the query resulted in extra results being requested From e27ceead55c4a81bed243ee1eec54b70e1b46ec0 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 16 Jun 2017 10:53:25 -0400 Subject: [PATCH 5/8] Fix test --- .../dstu3/ResourceProviderDstu3Test.java | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 5ee6a4d6d3b..ac20a5c6607 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -10,6 +10,8 @@ import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import static org.hamcrest.Matchers.stringContainsInOrder; @@ -1536,12 +1538,19 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { ourLog.info(myFhirCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(responseBundle)); - assertEquals(22, responseBundle.getEntry().size()); + List ids = new ArrayList(); + for (BundleEntryComponent nextEntry : responseBundle.getEntry()) { + ids.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + } + Collections.sort(ids); + ourLog.info("{} ids: {}", ids.size(), ids); + + assertThat(responseBundle.getEntry().size(), lessThanOrEqualTo(25)); - TreeSet ids = new TreeSet(); + TreeSet idsSet = new TreeSet(); for (int i = 0; i < responseBundle.getEntry().size(); i++) { for (BundleEntryComponent nextEntry : responseBundle.getEntry()) { - ids.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + idsSet.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); } } @@ -1549,7 +1558,7 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { responseBundle = ourClient.fetchResourceFromUrl(Bundle.class, nextUrl); for (int i = 0; i < responseBundle.getEntry().size(); i++) { for (BundleEntryComponent nextEntry : responseBundle.getEntry()) { - ids.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + idsSet.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); } } @@ -1557,19 +1566,19 @@ public class ResourceProviderDstu3Test extends BaseResourceProviderDstu3Test { responseBundle = ourClient.fetchResourceFromUrl(Bundle.class, nextUrl); for (int i = 0; i < responseBundle.getEntry().size(); i++) { for (BundleEntryComponent nextEntry : responseBundle.getEntry()) { - ids.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + idsSet.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); } } assertEquals(null, responseBundle.getLink("next")); - assertThat(ids, hasItem("List/A161444")); - assertThat(ids, hasItem("List/A161468")); - assertThat(ids, hasItem("List/A161500")); + assertThat(idsSet, hasItem("List/A161444")); + assertThat(idsSet, hasItem("List/A161468")); + assertThat(idsSet, hasItem("List/A161500")); ourLog.info("Expected {} - {}", allIds.size(), allIds); - ourLog.info("Actual {} - {}", ids.size(), ids); - assertEquals(allIds, ids); + ourLog.info("Actual {} - {}", idsSet.size(), idsSet); + assertEquals(allIds, idsSet); } From e6cb973f5f2f3381113ba9605c70bb262a491431 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Sat, 17 Jun 2017 08:29:30 -0400 Subject: [PATCH 6/8] Add test --- .../uhn/fhir/parser/JsonParserDstu3Test.java | 116 +++++++++------- .../src/test/resources/Patient.json.txt | 128 ++++++++++++++++++ 2 files changed, 196 insertions(+), 48 deletions(-) create mode 100644 hapi-fhir-structures-dstu3/src/test/resources/Patient.json.txt diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java index ac163282b6e..8ffee00417e 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/JsonParserDstu3Test.java @@ -46,6 +46,7 @@ import org.hl7.fhir.dstu3.model.CapabilityStatement.UnknownContentCode; import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender; import org.hl7.fhir.dstu3.model.Identifier.IdentifierUse; import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; +import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.utilities.xhtml.XhtmlNode; import org.junit.After; @@ -56,8 +57,10 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; +import com.google.common.base.Charsets; import com.google.common.collect.Sets; +import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; import ca.uhn.fhir.parser.IParserErrorHandler.IParseLocation; @@ -81,14 +84,14 @@ public class JsonParserDstu3Test { public void after() { ourCtx.setNarrativeGenerator(null); } - + /** * See #563 */ @Test public void testBadMessageForUnknownElement() throws IOException { String input = IOUtils.toString(JsonParserDstu3Test.class.getResourceAsStream("/bad_parse_bundle_1.json"), StandardCharsets.UTF_8); - + IParser p = ourCtx.newJsonParser(); p.setParserErrorHandler(new StrictErrorHandler()); try { @@ -99,14 +102,13 @@ public class JsonParserDstu3Test { } } - /** * See #563 */ @Test public void testBadMessageForUnknownElement2() throws IOException { String input = IOUtils.toString(JsonParserDstu3Test.class.getResourceAsStream("/bad_parse_bundle_2.json"), StandardCharsets.UTF_8); - + IParser p = ourCtx.newJsonParser(); p.setParserErrorHandler(new StrictErrorHandler()); try { @@ -116,35 +118,35 @@ public class JsonParserDstu3Test { assertEquals("Found incorrect type for element context - Expected OBJECT and found SCALAR (STRING)", e.getMessage()); } } - + /** * See #544 */ @Test public void testBundleStitchReferencesByUuid() throws Exception { Bundle bundle = new Bundle(); - + DocumentManifest dm = new DocumentManifest(); dm.getSubject().setReference("urn:uuid:96e85cca-9797-45d6-834a-c4eb27f331d3"); bundle.addEntry().setResource(dm); - + Patient patient = new Patient(); patient.addName().setFamily("FAMILY"); bundle.addEntry().setResource(patient).setFullUrl("urn:uuid:96e85cca-9797-45d6-834a-c4eb27f331d3"); - + String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(bundle); ourLog.info(encoded); - + bundle = ourCtx.newJsonParser().parseResource(Bundle.class, encoded); dm = (DocumentManifest) bundle.getEntry().get(0).getResource(); - + assertEquals("urn:uuid:96e85cca-9797-45d6-834a-c4eb27f331d3", dm.getSubject().getReference()); - + Patient subject = (Patient) dm.getSubject().getResource(); assertNotNull(subject); assertEquals("FAMILY", subject.getNameFirstRep().getFamily()); } - + /** * Test for the url generated based on the server config */ @@ -170,12 +172,12 @@ public class JsonParserDstu3Test { newPatient = jsonParser.parseResource(MyPatientWithCustomUrlExtension.class, new StringReader(parsedPatient)); assertEquals("myName", newPatient.getPetName().getValue()); - //Check no NPE if base server not configure + // Check no NPE if base server not configure newPatient = ourCtx.newJsonParser().parseResource(MyPatientWithCustomUrlExtension.class, new StringReader(parsedPatient)); assertNull("myName", newPatient.getPetName().getValue()); assertEquals("myName", ((StringType) newPatient.getExtensionsByUrl("http://www.example.com/petname").get(0).getValue()).getValue()); } - + @Test public void testCustomUrlExtensioninBundle() { final String expected = "{\"resourceType\":\"Bundle\",\"entry\":[{\"resource\":{\"resourceType\":\"Patient\",\"extension\":[{\"url\":\"http://www.example.com/petname\",\"valueString\":\"myName\"}]}}]}"; @@ -237,7 +239,6 @@ public class JsonParserDstu3Test { assertEquals(3, countMatches(encoded, "resourceType")); } - @Test public void testEncodeAndParseExtensions() throws Exception { @@ -323,7 +324,7 @@ public class JsonParserDstu3Test { assertEquals("CHILD", ((StringType) given2ext2.getValue()).getValue()); } - + @Test public void testEncodeAndParseMetaProfileAndTags() { Patient p = new Patient(); @@ -402,7 +403,6 @@ public class JsonParserDstu3Test { assertEquals("sec_label2", tagList.get(1).getDisplay()); } - /** * See #336 */ @@ -701,7 +701,7 @@ public class JsonParserDstu3Test { .addExtension() .setUrl("http://foo") .setValue(new Reference("Practitioner/A")); - + IParser parser = ourCtx.newJsonParser().setPrettyPrint(true); parser.setDontEncodeElements(new HashSet(Arrays.asList("*.id", "*.meta"))); @@ -1305,9 +1305,25 @@ public class JsonParserDstu3Test { assertEquals("{\"resourceType\":\"Observation\",\"valueQuantity\":{\"value\":0.0000000000000001}}", str); } + /** + * See #658 + */ + @Test + public void testExtraElement() throws Exception { + IParser p = ourCtx.newJsonParser(); + p.setParserErrorHandler(new StrictErrorHandler()); + try { + p.parseResource(IOUtils.toString(JsonParserDstu3Test.class.getResourceAsStream("/Patient.json.txt"), Charsets.UTF_8)); + fail(); + } catch (DataFormatException e) { + assertEquals("Found incorrect type for element assigner - Expected OBJECT and found SCALAR (STRING)", e.getMessage()); + } + + } + @Test public void testIncorrectJsonTypesIdAndArray() { - + // ID should be a String and communication should be an Array String input = "{\"resourceType\": \"Patient\",\n" + " \"id\": 123,\n" + @@ -1320,35 +1336,35 @@ public class JsonParserDstu3Test { "}"; IParser p = ourCtx.newJsonParser(); - + IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); p.setParserErrorHandler(errorHandler); Patient patient = (Patient) p.parseResource(input); - + ArgumentCaptor elementName = ArgumentCaptor.forClass(String.class); ArgumentCaptor found = ArgumentCaptor.forClass(ValueType.class); ArgumentCaptor expected = ArgumentCaptor.forClass(ValueType.class); ArgumentCaptor expectedScalarType = ArgumentCaptor.forClass(ScalarType.class); ArgumentCaptor foundScalarType = ArgumentCaptor.forClass(ScalarType.class); verify(errorHandler, times(2)).incorrectJsonType(any(IParseLocation.class), elementName.capture(), expected.capture(), expectedScalarType.capture(), found.capture(), foundScalarType.capture()); - + assertEquals(ValueType.SCALAR, found.getAllValues().get(0)); assertEquals(ValueType.SCALAR, expected.getAllValues().get(0)); assertEquals(ScalarType.NUMBER, foundScalarType.getAllValues().get(0)); assertEquals(ScalarType.STRING, expectedScalarType.getAllValues().get(0)); - + assertEquals(ValueType.OBJECT, found.getAllValues().get(1)); assertEquals(ValueType.ARRAY, expected.getAllValues().get(1)); assertEquals(null, foundScalarType.getAllValues().get(1)); assertEquals(null, expectedScalarType.getAllValues().get(1)); - + assertEquals("123", patient.getIdElement().getIdPart()); assertEquals("Hindi", patient.getCommunicationFirstRep().getLanguage().getText()); } @Test public void testIncorrectJsonTypesNone() { - + // ID should be a String and communication should be an Array String input = "{\"resourceType\": \"Patient\",\n" + " \"id\": \"123\",\n" + @@ -1361,18 +1377,18 @@ public class JsonParserDstu3Test { "}"; IParser p = ourCtx.newJsonParser(); - + IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); p.setParserErrorHandler(errorHandler); Patient patient = (Patient) p.parseResource(input); - + ArgumentCaptor elementName = ArgumentCaptor.forClass(String.class); ArgumentCaptor found = ArgumentCaptor.forClass(ValueType.class); ArgumentCaptor expected = ArgumentCaptor.forClass(ValueType.class); ArgumentCaptor expectedScalarType = ArgumentCaptor.forClass(ScalarType.class); ArgumentCaptor foundScalarType = ArgumentCaptor.forClass(ScalarType.class); verify(errorHandler, times(0)).incorrectJsonType(any(IParseLocation.class), elementName.capture(), expected.capture(), expectedScalarType.capture(), found.capture(), foundScalarType.capture()); - + assertEquals("123", patient.getIdElement().getIdPart()); assertEquals("Hindi", patient.getCommunicationFirstRep().getLanguage().getText()); } @@ -1380,21 +1396,21 @@ public class JsonParserDstu3Test { @Test public void testInvalidDateTimeValueInvalid() throws Exception { IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); - + String res = "{ \"resourceType\": \"Observation\", \"valueDateTime\": \"foo\" }"; IParser parser = ourCtx.newJsonParser(); parser.setParserErrorHandler(errorHandler); Observation parsed = parser.parseResource(Observation.class, res); - + assertEquals(null, parsed.getValueDateTimeType().getValue()); assertEquals("foo", parsed.getValueDateTimeType().getValueAsString()); - + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); verify(errorHandler, times(1)).invalidValue(isNull(IParseLocation.class), eq("foo"), msgCaptor.capture()); assertEquals("Invalid date/time format: \"foo\"", msgCaptor.getValue()); - + String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); - assertEquals("{\"resourceType\":\"Observation\",\"valueDateTime\":\"foo\"}", encoded); + assertEquals("{\"resourceType\":\"Observation\",\"valueDateTime\":\"foo\"}", encoded); } /** @@ -1412,19 +1428,19 @@ public class JsonParserDstu3Test { @Test public void testInvalidEnumValueBlank() { IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); - + String res = "{ \"resourceType\": \"Patient\", \"gender\": \"\" }"; IParser parser = ourCtx.newJsonParser(); parser.setParserErrorHandler(errorHandler); Patient parsed = parser.parseResource(Patient.class, res); - + assertEquals(null, parsed.getGenderElement().getValue()); assertEquals(null, parsed.getGenderElement().getValueAsString()); - + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); verify(errorHandler, times(1)).invalidValue(isNull(IParseLocation.class), eq(""), msgCaptor.capture()); assertEquals("Attribute values must not be empty (\"\")", msgCaptor.getValue()); - + String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); assertEquals("{\"resourceType\":\"Patient\"}", encoded); } @@ -1432,21 +1448,21 @@ public class JsonParserDstu3Test { @Test public void testInvalidEnumValueInvalid() { IParserErrorHandler errorHandler = mock(IParserErrorHandler.class); - + String res = "{ \"resourceType\": \"Patient\", \"gender\": \"foo\" }"; IParser parser = ourCtx.newJsonParser(); parser.setParserErrorHandler(errorHandler); Patient parsed = parser.parseResource(Patient.class, res); - + assertEquals(null, parsed.getGenderElement().getValue()); assertEquals("foo", parsed.getGenderElement().getValueAsString()); - + ArgumentCaptor msgCaptor = ArgumentCaptor.forClass(String.class); verify(errorHandler, times(1)).invalidValue(isNull(IParseLocation.class), eq("foo"), msgCaptor.capture()); assertEquals("Unknown AdministrativeGender code 'foo'", msgCaptor.getValue()); - + String encoded = ourCtx.newJsonParser().encodeResourceToString(parsed); - assertEquals("{\"resourceType\":\"Patient\",\"gender\":\"foo\"}", encoded); + assertEquals("{\"resourceType\":\"Patient\",\"gender\":\"foo\"}", encoded); } /** @@ -1910,10 +1926,10 @@ public class JsonParserDstu3Test { public void testParseEmptyValue() { String input = "{\"resourceType\":\"QuestionnaireResponse\",\"id\":\"123\",\"authored\":\"\",\"group\":{\"linkId\":\"\"}}"; IParser parser = ourCtx.newJsonParser(); - + parser.setParserErrorHandler(new LenientErrorHandler().setErrorOnInvalidValue(false)); QuestionnaireResponse qr = parser.parseResource(QuestionnaireResponse.class, input); - + assertEquals("QuestionnaireResponse/123", qr.getIdElement().getValue()); assertEquals(null, qr.getAuthored()); assertEquals(null, qr.getAuthoredElement().getValue()); @@ -2298,10 +2314,14 @@ public class JsonParserDstu3Test { ArgumentCaptor actual = ArgumentCaptor.forClass(ValueType.class); ArgumentCaptor expectedScalar = ArgumentCaptor.forClass(ScalarType.class); ArgumentCaptor actualScalar = ArgumentCaptor.forClass(ScalarType.class); - verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), elementName.capture(), expected.capture(), expectedScalar.capture(), actual.capture(), actualScalar.capture()); - verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("_id"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), Mockito.eq(ValueType.SCALAR), actualScalar.capture()); - verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("__v"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), Mockito.eq(ValueType.SCALAR), actualScalar.capture()); - verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("_status"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), Mockito.eq(ValueType.SCALAR), actualScalar.capture()); + verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), elementName.capture(), expected.capture(), expectedScalar.capture(), actual.capture(), + actualScalar.capture()); + verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("_id"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), Mockito.eq(ValueType.SCALAR), + actualScalar.capture()); + verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("__v"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), Mockito.eq(ValueType.SCALAR), + actualScalar.capture()); + verify(errorHandler, atLeastOnce()).incorrectJsonType(Mockito.any(IParseLocation.class), Mockito.eq("_status"), Mockito.eq(ValueType.OBJECT), expectedScalar.capture(), + Mockito.eq(ValueType.SCALAR), actualScalar.capture()); assertEquals("_id", elementName.getAllValues().get(0)); assertEquals(ValueType.OBJECT, expected.getAllValues().get(0)); diff --git a/hapi-fhir-structures-dstu3/src/test/resources/Patient.json.txt b/hapi-fhir-structures-dstu3/src/test/resources/Patient.json.txt new file mode 100644 index 00000000000..75f6cbcbc6f --- /dev/null +++ b/hapi-fhir-structures-dstu3/src/test/resources/Patient.json.txt @@ -0,0 +1,128 @@ +{ + "resourceType": "Patient", + "id": "patient1", + "contained": [{ + "resourceType": "Practitioner", + "id": "p1", + "identifier": [{ + "system": "urn:oid:2.16.840.1.113883.3.566", + "value": "145148", + "assigner": "UHC" + }], + "name": [{ + "text": " Richard J Y Ha MD", + "given": ["Richard", + "J Y"], + "family": "Ha", + "suffix": ["MD"] + }] + }], + "identifier": [{ + "use": "official", + "type": { + "coding": [{ + "system": "http://hl7.org/fhir/identifier-type", + "code": "MR", + "display": "Medical Record Number", + "userSelected": true + }] + }, + "system": "urn:oid:2.16.840.1.113883.3.566", + "value": "220457511", + "assigner": "MU" + }, + { + "type": { + "coding": [{ + "system": "http://hl7.org/fhir/identifier-type", + "code": "MR", + "display": "UHC ID", + "userSelected": true + }] + }, + "system": "urn:oid:2.16.840.1.113883.3.566", + "value": "15246931", + "assigner": "UHC" + }, + { + "type": { + "coding": [{ + "system": "http://hl7.org/fhir/identifier-type", + "code": "MR", + "display": "ACCT NUM", + "userSelected": true + }] + }, + "system": "urn:oid:2.16.840.1.113883.3.566", + "value": "226274321", + "assigner": "MU" + }, + { + "type": { + "coding": [{ + "system": "http://hl7.org/fhir/identifier-type", + "code": "MR", + "display": "HNAMPERSON_ID", + "userSelected": true + }] + }, + "system": "urn:oid:2.16.840.1.113883.3.566", + "value": "25296343" + }], + "active": true, + "name": [{ + "use": "official", + "text": " MOM TWO CERNER", + "given": ["MOM", + "TWO"], + "family": "CERNER" + }], + "gender": "female", + "birthDate": "1990-10-18", + "address": [{ + "use": "home", + "text": "2401 LEMONE INDUSTRIAL BLVD, COLUMBIA, MO 65201 ", + "line": "2401 LEMONE INDUSTRIAL BLVD", + "city": "COLUMBIA", + "state": "MO", + "postalCode": "65201" + }], + "maritalStatus": { + "coding": [{ + "system": "urn:oid:2.16.840.1.113883.3.566", + "code": "M", + "display": "Married", + "userSelected": true + }] + }, + "contact": [{ + "name": { + "text": "BABY TWO CERNER", + "given": ["BABY", + "TWO"], + "family": "CERNER" + }, + "address": { + "text": "2401 LEMONE INDUSTRIAL BLVD, COLUMBIA MO 65201 ", + "line": "2401 LEMONE INDUSTRIAL BLVD", + "city": "COLUMBIA", + "state": "MO", + "postalCode": "65201" + } + }], + "communication": [{ + "language": { + "coding": [{ + "system": "urn:oid:2.16.840.1.113883.3.566", + "code": "ENG", + "userSelected": true + }] + } + }], + "generalPractitioner": [{ + "reference": "#p1" + }], + "managingOrganization": { + "display": "Organization/UHC" + } +} \ No newline at end of file From f5a3ad67518e94cd57f363e80107d61e98673a1a Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 19 Jun 2017 13:56:25 -0400 Subject: [PATCH 7/8] Note for #674 --- src/changes/changes.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d0cf8a64659..2adefcf2365 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -48,6 +48,10 @@ performance problems in situations where lare queries should not be needed + + Prevent duplicates in $everything query response in JPA server. Thanks to @vlad-ignatov + for reporting! + From e147cf321de7dff24f798d888da89251b30959d7 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 19 Jun 2017 13:56:38 -0400 Subject: [PATCH 8/8] Fix #674 - Avoid duplicates in $everything query Squashed commit of the following: commit f3097f423f5f1e1d27f4084aa4ab0daa01618149 Author: James Agnew Date: Mon Jun 19 13:24:29 2017 -0400 more travis fun commit a4b8161597057562d03c2abb3eed3c56dae874ba Author: James Agnew Date: Mon Jun 19 10:43:33 2017 -0400 More fighting with travis commit fe47d1e8643ae0b7860567ad02db50aac42c509b Author: James Agnew Date: Mon Jun 19 10:10:55 2017 -0400 More travis attempts commit 4fdfe7a4e81ff28407209f7b03a37cfddf946586 Author: James Agnew Date: Mon Jun 19 09:25:04 2017 -0400 Try and run unit tests in 2 threads to cut time.. Will travis like this? commit 571045b63da04149397acfad6798193b384e541f Author: James Date: Mon Jun 19 07:35:46 2017 -0400 Paging now working commit 526a1fa7d03f9ca0d1c4a3921fb7ea0faa783c4c Merge: cebe881a15 55a67ae055 Author: James Agnew Date: Mon Jun 19 06:19:37 2017 -0400 Merge branch '674_everything_improvements' of github.com:jamesagnew/hapi-fhir into 674_everything_improvements commit cebe881a158cfbbf0adb42607029862a39e1e169 Merge: b3b9273ca7 5789cd2a46 Author: James Agnew Date: Mon Jun 19 06:19:12 2017 -0400 Merge branch 'master' into 674_everything_improvements for #674 commit b3b9273ca74a7993bfef362710727c6aa2c05756 Author: James Agnew Date: Mon Jun 19 06:16:27 2017 -0400 Work on everything fixes for #674 commit 55a67ae05509b197bff4f4d0c331da334a58b85d Author: James Agnew Date: Mon Jun 19 06:16:27 2017 -0400 Work on everything fixes --- .travis.yml | 5 +- hapi-fhir-jpaserver-base/pom.xml | 24 +- .../fhir/jpa/dao/BaseHapiFhirSystemDao.java | 1 + .../java/ca/uhn/fhir/jpa/dao/DaoConfig.java | 36 ++- .../ca/uhn/fhir/jpa/dao/SearchBuilder.java | 201 ++++++++++++-- .../search/PersistedJpaBundleProvider.java | 2 +- .../dstu3/PatientEverythingDstu3Test.java | 254 ++++++++++++++++++ .../dstu3/ResourceProviderDstu3Test.java | 1 + pom.xml | 14 + src/site/fml/hapi-fhir-faq.fml | 22 ++ 10 files changed, 522 insertions(+), 38 deletions(-) create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java diff --git a/.travis.yml b/.travis.yml index 5c38a56f108..4b5b599e73a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,6 @@ # Use docker-based build environment (instead of openvz) -sudo: false +#sudo: false +sudo: required language: java jdk: @@ -21,4 +22,4 @@ before_script: script: # - mvn -e -B clean install && cd hapi-fhir-ra && mvn -e -B -DTRAVIS_JOB_ID=$TRAVIS_JOB_ID clean test jacoco:report coveralls:report # - mvn -Dci=true -e -B -P ALLMODULES,NOPARALLEL,ERRORPRONE clean install && cd hapi-fhir-jacoco && mvn -e -B -DTRAVIS_JOB_ID=$TRAVIS_JOB_ID jacoco:report coveralls:report - - mvn -Dci=true -e -B -P ALLMODULES,NOPARALLEL clean install && cd hapi-fhir-jacoco && mvn -e -B -DTRAVIS_JOB_ID=$TRAVIS_JOB_ID jacoco:report coveralls:report + - mvn -Dci=true -e -B -P ALLMODULES,MINPARALLEL clean install && cd hapi-fhir-jacoco && mvn -e -B -DTRAVIS_JOB_ID=$TRAVIS_JOB_ID jacoco:report coveralls:report diff --git a/hapi-fhir-jpaserver-base/pom.xml b/hapi-fhir-jpaserver-base/pom.xml index 365b4a34a89..3488e8e3caa 100644 --- a/hapi-fhir-jpaserver-base/pom.xml +++ b/hapi-fhir-jpaserver-base/pom.xml @@ -632,13 +632,21 @@ - - + + MINPARALLEL + + + + org.apache.maven.plugins + maven-surefire-plugin + + 2 + true + + + + + + diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java index 3e0ae8af35b..7997844ac88 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirSystemDao.java @@ -170,6 +170,7 @@ public abstract class BaseHapiFhirSystemDao extends BaseHapiFhirDao getResourceCounts() { CriteriaBuilder builder = myEntityManager.getCriteriaBuilder(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java index 56aee54fbd7..475599260bd 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/DaoConfig.java @@ -87,15 +87,16 @@ public class DaoConfig { private boolean myEnforceReferentialIntegrityOnWrite = true; + private int myEverythingIncludesFetchPageSize = 50; /** * update setter javadoc if default changes */ private long myExpireSearchResultsAfterMillis = DateUtils.MILLIS_PER_HOUR; + /** * update setter javadoc if default changes */ private Integer myFetchSizeDefaultMaximum = null; - private int myHardTagListLimit = 1000; private int myIncludeLimit = 2000; /** @@ -147,6 +148,22 @@ public class DaoConfig { return myDeferIndexingForCodesystemsOfSize; } + /** + * Unlike with normal search queries, $everything queries have their _includes loaded by the main search thread and these included results + * are added to the normal search results instead of being added on as extras in a page. This means that they will not appear multiple times + * as the search results are paged over. + *

+ * In order to recursively load _includes, we process the original results in batches of this size. Adjust with caution, increasing this + * value may improve performance but may also cause memory issues. + *

+ *

+ * The default value is 50 + *

+ */ + public int getEverythingIncludesFetchPageSize() { + return myEverythingIncludesFetchPageSize; + } + /** * Sets the number of milliseconds that search results for a given client search * should be preserved before being purged from the database. @@ -537,6 +554,23 @@ public class DaoConfig { myEnforceReferentialIntegrityOnWrite = theEnforceReferentialIntegrityOnWrite; } + /** + * Unlike with normal search queries, $everything queries have their _includes loaded by the main search thread and these included results + * are added to the normal search results instead of being added on as extras in a page. This means that they will not appear multiple times + * as the search results are paged over. + *

+ * In order to recursively load _includes, we process the original results in batches of this size. Adjust with caution, increasing this + * value may improve performance but may also cause memory issues. + *

+ *

+ * The default value is 50 + *

+ */ + public void setEverythingIncludesFetchPageSize(int theEverythingIncludesFetchPageSize) { + Validate.inclusiveBetween(1, Integer.MAX_VALUE, theEverythingIncludesFetchPageSize); + myEverythingIncludesFetchPageSize = theEverythingIncludesFetchPageSize; + } + /** * If this is set to false (default is true) the stale search deletion * task will be disabled (meaning that search results will be retained in the database indefinitely). USE WITH CAUTION. 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 e2d6223fa51..36f99d9368e 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 @@ -27,31 +27,86 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; import java.math.BigDecimal; import java.math.MathContext; -import java.util.*; +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.*; +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.*; +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.hl7.fhir.instance.model.api.*; +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.collect.*; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; -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.FhirVersionEnum; +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.IResourceIndexedSearchParamUriDao; -import ca.uhn.fhir.jpa.entity.*; +import ca.uhn.fhir.jpa.entity.BaseHasResource; +import ca.uhn.fhir.jpa.entity.BaseResourceIndexedSearchParam; +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.term.IHapiTerminologySvc; import ca.uhn.fhir.jpa.term.VersionIndependentConcept; import ca.uhn.fhir.jpa.util.StopWatch; -import ca.uhn.fhir.model.api.*; -import ca.uhn.fhir.model.base.composite.*; +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; import ca.uhn.fhir.model.dstu.resource.BaseResource; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.primitive.InstantDt; @@ -60,9 +115,23 @@ import ca.uhn.fhir.parser.DataFormatException; import ca.uhn.fhir.rest.api.SortOrderEnum; import ca.uhn.fhir.rest.api.SortSpec; import ca.uhn.fhir.rest.method.RestSearchParameterTypeEnum; -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.Constants; -import ca.uhn.fhir.rest.server.exceptions.*; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; +import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.util.UrlUtil; /** @@ -70,6 +139,8 @@ import ca.uhn.fhir.util.UrlUtil; * searchs for resources */ public class SearchBuilder implements ISearchBuilder { + private static final List EMPTY_LONG_LIST = Collections.unmodifiableList(new ArrayList()); + private static Long NO_MORE = Long.valueOf(-1); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchBuilder.class); private List myAlsoIncludePids; @@ -1048,7 +1119,7 @@ public class SearchBuilder implements ISearchBuilder { if (!isBlank(unitsValue)) { code = theBuilder.equal(theFrom.get("myUnits"), unitsValue); } - + cmpValue = ObjectUtils.defaultIfNull(cmpValue, ParamPrefixEnum.EQUAL); final Expression path = theFrom.get("myValue"); String invalidMessageName = "invalidQuantityPrefix"; @@ -1357,11 +1428,11 @@ public class SearchBuilder implements ISearchBuilder { * Now perform the search */ final TypedQuery query = myEntityManager.createQuery(outerQuery); - + if (theMaximumResults != null) { query.setMaxResults(theMaximumResults); } - + return query; } @@ -1629,12 +1700,6 @@ public class SearchBuilder implements ISearchBuilder { List results = q.getResultList(); for (ResourceLink resourceLink : results) { if (theReverseMode) { - // if (theEverythingModeEnum.isEncounter()) { - // if (resourceLink.getSourcePath().equals("Encounter.subject") || - // resourceLink.getSourcePath().equals("Encounter.patient")) { - // nextRoundOmit.add(resourceLink.getSourceResourcePid()); - // } - // } pidsToInclude.add(resourceLink.getSourceResourcePid()); } else { pidsToInclude.add(resourceLink.getTargetResourcePid()); @@ -1745,10 +1810,10 @@ public class SearchBuilder implements ISearchBuilder { } private void searchForIdsWithAndOr(String theResourceName, String theParamName, List> theAndOrParams) { - + for (int andListIdx = 0; andListIdx < theAndOrParams.size(); andListIdx++) { List nextOrList = theAndOrParams.get(andListIdx); - + for (int orListIdx = 0; orListIdx < nextOrList.size(); orListIdx++) { IQueryParameterType nextOr = nextOrList.get(orListIdx); boolean hasNoValue = false; @@ -1760,14 +1825,14 @@ public class SearchBuilder implements ISearchBuilder { hasNoValue = true; } } - + if (hasNoValue) { ourLog.debug("Ignoring empty parameter: {}", theParamName); nextOrList.remove(orListIdx); orListIdx--; } } - + if (nextOrList.isEmpty()) { theAndOrParams.remove(andListIdx); andListIdx--; @@ -1777,7 +1842,7 @@ public class SearchBuilder implements ISearchBuilder { if (theAndOrParams.isEmpty()) { return; } - + if (theParamName.equals(BaseResource.SP_RES_ID)) { addPredicateResourceId(theAndOrParams); @@ -1972,6 +2037,63 @@ public class SearchBuilder implements ISearchBuilder { static Predicate[] toArray(List thePredicates) { return thePredicates.toArray(new Predicate[thePredicates.size()]); } + public class IncludesIterator implements Iterator{ + + private Iterator myCurrentIterator; + private int myCurrentOffset; + private ArrayList myCurrentPids; + private Long myNext; + private int myPageSize = myCallingDao.getConfig().getEverythingIncludesFetchPageSize(); + + public IncludesIterator(Set thePidSet) { + myCurrentPids = new ArrayList(thePidSet); + myCurrentIterator = EMPTY_LONG_LIST.iterator(); + myCurrentOffset = 0; + } + + private void fetchNext() { + while (myNext == null) { + + if (myCurrentIterator.hasNext()) { + myNext = myCurrentIterator.next(); + break; + } + + if (!myCurrentIterator.hasNext()) { + int start = myCurrentOffset; + int end = myCurrentOffset + myPageSize; + if (end > myCurrentPids.size()) { + end = myCurrentPids.size(); + } + if (end - start <= 0) { + myNext = NO_MORE; + break; + } + myCurrentOffset = end; + Collection pidsToScan = myCurrentPids.subList(start, end); + Set includes = Collections.singleton(new Include("*", true)); + Set newPids = loadReverseIncludes(myCallingDao, myContext, myEntityManager, pidsToScan, includes, false, myParams.getLastUpdated()); + myCurrentIterator = newPids.iterator(); + } + + } + } + + @Override + public boolean hasNext() { + fetchNext(); + return myNext != NO_MORE; + } + + @Override + public Long next() { + fetchNext(); + Long retVal = myNext; + myNext = null; + return retVal; + } + + } private enum JoinEnum { DATE, NUMBER, QUANTITY, REFERENCE, STRING, TOKEN, URI @@ -2007,16 +2129,24 @@ public class SearchBuilder implements ISearchBuilder { } private final class QueryIterator implements Iterator { + private boolean myFirst = true; + private IncludesIterator myIncludesIterator; private Long myNext; private final Set myPidSet = new HashSet(); private Iterator myPreResultsIterator; private Iterator myResultsIterator; private SortSpec mySort; + private boolean myStillNeedToFetchIncludes; private StopWatch myStopwatch = null; private QueryIterator() { mySort = myParams.getSort(); + + // Includes are processed inline for $everything query + if (myParams.getEverythingMode() != null) { + myStillNeedToFetchIncludes = true; + } } private void fetchNext() { @@ -2061,7 +2191,24 @@ public class SearchBuilder implements ISearchBuilder { } if (myNext == null) { - myNext = NO_MORE; + if (myStillNeedToFetchIncludes) { + myIncludesIterator = new IncludesIterator(myPidSet); + myStillNeedToFetchIncludes = false; + } + if (myIncludesIterator != null) { + while (myIncludesIterator.hasNext()) { + Long next = myIncludesIterator.next(); + if (next != null && myPidSet.add(next)) { + myNext = next; + break; + } + } + if (myNext == null) { + myNext = NO_MORE; + } + } else { + myNext = NO_MORE; + } } } // if we need to fetch the next result @@ -2070,9 +2217,11 @@ public class SearchBuilder implements ISearchBuilder { ourLog.info("Initial query result returned in {}ms for query {}", myStopwatch.getMillis(), mySearchUuid); myFirst = false; } + if (myNext == NO_MORE) { ourLog.info("Query found {} matches in {}ms for query {}", myPidSet.size(), myStopwatch.getMillis(), mySearchUuid); } + } @Override diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java index 083d76d38a2..d077dea033a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/PersistedJpaBundleProvider.java @@ -241,8 +241,8 @@ public class PersistedJpaBundleProvider implements IBundleProvider { Set includedPids = new HashSet(); if (mySearchEntity.getSearchType() == SearchTypeEnum.SEARCH) { includedPids.addAll(sb.loadReverseIncludes(myDao, myContext, myEntityManager, pidsSubList, mySearchEntity.toRevIncludesList(), true, mySearchEntity.getLastUpdated())); + includedPids.addAll(sb.loadReverseIncludes(myDao, myContext, myEntityManager, pidsSubList, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated())); } - includedPids.addAll(sb.loadReverseIncludes(myDao, myContext, myEntityManager, pidsSubList, mySearchEntity.toIncludesList(), false, mySearchEntity.getLastUpdated())); // Execute the query and make sure we return distinct results List resources = new ArrayList(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java new file mode 100644 index 00000000000..1f65387acc4 --- /dev/null +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/PatientEverythingDstu3Test.java @@ -0,0 +1,254 @@ +package ca.uhn.fhir.jpa.provider.dstu3; + +import static org.apache.commons.lang3.StringUtils.isNotBlank; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsInRelativeOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasItems; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.lessThanOrEqualTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; +import static org.hamcrest.Matchers.stringContainsInOrder; +import static org.junit.Assert.*; + +import java.io.*; +import java.net.*; +import java.nio.charset.StandardCharsets; +import java.util.*; + +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Validate; +import org.apache.http.NameValuePair; +import org.apache.http.client.ClientProtocolException; +import org.apache.http.client.entity.UrlEncodedFormEntity; +import org.apache.http.client.methods.*; +import org.apache.http.entity.*; +import org.apache.http.message.BasicNameValuePair; +import org.hl7.fhir.dstu3.model.*; +import org.hl7.fhir.dstu3.model.Bundle.*; +import org.hl7.fhir.dstu3.model.Encounter.EncounterLocationComponent; +import org.hl7.fhir.dstu3.model.Encounter.EncounterStatus; +import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender; +import org.hl7.fhir.dstu3.model.Narrative.NarrativeStatus; +import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; +import org.hl7.fhir.dstu3.model.Questionnaire.QuestionnaireItemType; +import org.hl7.fhir.dstu3.model.Subscription.SubscriptionChannelType; +import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus; +import org.hl7.fhir.instance.model.Encounter.EncounterState; +import org.hl7.fhir.instance.model.api.*; +import org.junit.*; +import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.support.TransactionCallback; + +import com.google.common.base.Charsets; +import com.google.common.collect.Lists; + +import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.entity.Search; +import ca.uhn.fhir.model.api.TemporalPrecisionEnum; +import ca.uhn.fhir.model.primitive.InstantDt; +import ca.uhn.fhir.model.primitive.UriDt; +import ca.uhn.fhir.parser.IParser; +import ca.uhn.fhir.parser.StrictErrorHandler; +import ca.uhn.fhir.rest.api.MethodOutcome; +import ca.uhn.fhir.rest.api.SummaryEnum; +import ca.uhn.fhir.rest.client.IGenericClient; +import ca.uhn.fhir.rest.gclient.StringClientParam; +import ca.uhn.fhir.rest.param.*; +import ca.uhn.fhir.rest.server.Constants; +import ca.uhn.fhir.rest.server.EncodingEnum; +import ca.uhn.fhir.rest.server.exceptions.*; +import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; +import ca.uhn.fhir.util.TestUtil; +import ca.uhn.fhir.util.UrlUtil; + +public class PatientEverythingDstu3Test extends BaseResourceProviderDstu3Test { + + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(PatientEverythingDstu3Test.class); + private String orgId; + private String patId; + private String encId1; + private String encId2; + private ArrayList myObsIds; + private String myWrongPatId; + private String myWrongEnc1; + + @Before + public void beforeDisableResultReuse() { + myDaoConfig.setReuseCachedSearchResultsForMillis(null); + } + + @Override + @After + public void after() throws Exception { + super.after(); + + myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); + myDaoConfig.setEverythingIncludesFetchPageSize(new DaoConfig().getEverythingIncludesFetchPageSize()); + } + + @Override + public void before() throws Exception { + super.before(); + myFhirCtx.setParserErrorHandler(new StrictErrorHandler()); + + myDaoConfig.setAllowMultipleDelete(true); + + Organization org = new Organization(); + org.setName("an org"); + orgId = ourClient.create().resource(org).execute().getId().toUnqualifiedVersionless().getValue(); + ourLog.info("OrgId: {}", orgId); + + Patient patient = new Patient(); + patient.getManagingOrganization().setReference(orgId); + patId = ourClient.create().resource(patient).execute().getId().toUnqualifiedVersionless().getValue(); + + Patient patient2 = new Patient(); + patient2.getManagingOrganization().setReference(orgId); + myWrongPatId = ourClient.create().resource(patient2).execute().getId().toUnqualifiedVersionless().getValue(); + + Encounter enc1 = new Encounter(); + enc1.setStatus(EncounterStatus.CANCELLED); + enc1.getSubject().setReference(patId); + enc1.getServiceProvider().setReference(orgId); + encId1 = ourClient.create().resource(enc1).execute().getId().toUnqualifiedVersionless().getValue(); + + Encounter enc2 = new Encounter(); + enc2.setStatus(EncounterStatus.ARRIVED); + enc2.getSubject().setReference(patId); + enc2.getServiceProvider().setReference(orgId); + encId2 = ourClient.create().resource(enc2).execute().getId().toUnqualifiedVersionless().getValue(); + + Encounter wrongEnc1 = new Encounter(); + wrongEnc1.setStatus(EncounterStatus.ARRIVED); + wrongEnc1.getSubject().setReference(myWrongPatId); + wrongEnc1.getServiceProvider().setReference(orgId); + myWrongEnc1 = ourClient.create().resource(wrongEnc1).execute().getId().toUnqualifiedVersionless().getValue(); + + myObsIds = new ArrayList(); + for (int i = 0; i < 20; i++) { + Observation obs = new Observation(); + obs.getSubject().setReference(patId); + obs.setStatus(ObservationStatus.FINAL); + String obsId = ourClient.create().resource(obs).execute().getId().toUnqualifiedVersionless().getValue(); + myObsIds.add(obsId); + } + + } + + /** + * See #674 + */ + @Test + public void testEverythingReturnsCorrectResources() throws Exception { + + Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=json&_count=100", EncodingEnum.JSON); + + assertNull(bundle.getLink("next")); + + Set actual = new TreeSet(); + for (BundleEntryComponent nextEntry : bundle.getEntry()) { + actual.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + } + + ourLog.info("Found IDs: {}", actual); + + assertThat(actual, hasItem(patId)); + assertThat(actual, hasItem(encId1)); + assertThat(actual, hasItem(encId2)); + assertThat(actual, hasItem(orgId)); + assertThat(actual, hasItems(myObsIds.toArray(new String[0]))); + assertThat(actual, not(hasItem(myWrongPatId))); + assertThat(actual, not(hasItem(myWrongEnc1))); + } + + /** + * See #674 + */ + @Test + public void testEverythingReturnsCorrectResourcesSmallPage() throws Exception { + myDaoConfig.setEverythingIncludesFetchPageSize(1); + + Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=json&_count=100", EncodingEnum.JSON); + + assertNull(bundle.getLink("next")); + + Set actual = new TreeSet(); + for (BundleEntryComponent nextEntry : bundle.getEntry()) { + actual.add(nextEntry.getResource().getIdElement().toUnqualifiedVersionless().getValue()); + } + + ourLog.info("Found IDs: {}", actual); + + assertThat(actual, hasItem(patId)); + assertThat(actual, hasItem(encId1)); + assertThat(actual, hasItem(encId2)); + assertThat(actual, hasItem(orgId)); + assertThat(actual, hasItems(myObsIds.toArray(new String[0]))); + assertThat(actual, not(hasItem(myWrongPatId))); + assertThat(actual, not(hasItem(myWrongEnc1))); + } + + /** + * See #674 + */ + @Test + public void testEverythingPagesWithCorrectEncodingJson() throws Exception { + + Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=json&_count=1", EncodingEnum.JSON); + + assertNotNull(bundle.getLink("next").getUrl()); + assertThat(bundle.getLink("next").getUrl(), containsString("_format=json")); + bundle = fetchBundle(bundle.getLink("next").getUrl(), EncodingEnum.JSON); + + assertNotNull(bundle.getLink("next").getUrl()); + assertThat(bundle.getLink("next").getUrl(), containsString("_format=json")); + bundle = fetchBundle(bundle.getLink("next").getUrl(), EncodingEnum.JSON); + } + + /** + * See #674 + */ + @Test + public void testEverythingPagesWithCorrectEncodingXml() throws Exception { + + Bundle bundle = fetchBundle(ourServerBase + "/" + patId + "/$everything?_format=xml&_count=1", EncodingEnum.XML); + + assertNotNull(bundle.getLink("next").getUrl()); + ourLog.info("Next link: {}", bundle.getLink("next").getUrl()); + assertThat(bundle.getLink("next").getUrl(), containsString("_format=xml")); + bundle = fetchBundle(bundle.getLink("next").getUrl(), EncodingEnum.XML); + + assertNotNull(bundle.getLink("next").getUrl()); + ourLog.info("Next link: {}", bundle.getLink("next").getUrl()); + assertThat(bundle.getLink("next").getUrl(), containsString("_format=xml")); + bundle = fetchBundle(bundle.getLink("next").getUrl(), EncodingEnum.XML); + } + + private Bundle fetchBundle(String theUrl, EncodingEnum theEncoding) throws IOException, ClientProtocolException { + Bundle bundle; + HttpGet get = new HttpGet(theUrl); + CloseableHttpResponse resp = ourHttpClient.execute(get); + try { + assertEquals(theEncoding.getResourceContentTypeNonLegacy(), resp.getFirstHeader(Constants.HEADER_CONTENT_TYPE).getValue().replaceAll(";.*", "")); + bundle = theEncoding.newParser(myFhirCtx).parseResource(Bundle.class, IOUtils.toString(resp.getEntity().getContent(), Charsets.UTF_8)); + } finally { + IOUtils.closeQuietly(resp); + } + + return bundle; + } + + @AfterClass + public static void afterClassClearContext() { + TestUtil.clearAllStaticFieldsForUnitTest(); + } + +} diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java index 585058e00a2..c4a74a2c204 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/dstu3/ResourceProviderDstu3Test.java @@ -47,6 +47,7 @@ import org.hl7.fhir.dstu3.model.Observation.ObservationStatus; import org.hl7.fhir.dstu3.model.Questionnaire.QuestionnaireItemType; import org.hl7.fhir.dstu3.model.Subscription.SubscriptionChannelType; import org.hl7.fhir.dstu3.model.Subscription.SubscriptionStatus; +import org.hl7.fhir.instance.model.Encounter.EncounterState; import org.hl7.fhir.instance.model.api.*; import org.junit.*; import org.springframework.transaction.TransactionStatus; diff --git a/pom.xml b/pom.xml index 9d8cb8cd28d..084e85d2858 100644 --- a/pom.xml +++ b/pom.xml @@ -1774,6 +1774,20 @@ + + MINPARALLEL + + + + org.apache.maven.plugins + maven-surefire-plugin + + 2 + + + + + ERRORPRONE diff --git a/src/site/fml/hapi-fhir-faq.fml b/src/site/fml/hapi-fhir-faq.fml index 87d90f4013e..4a9303b9623 100644 --- a/src/site/fml/hapi-fhir-faq.fml +++ b/src/site/fml/hapi-fhir-faq.fml @@ -145,4 +145,26 @@ + + Contributing + + + My build is failing with the following error: + [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test (default-test) on project hapi-fhir-jpaserver-base: Execution default-test of goal org.apache.maven.plugins:maven-surefire-plugin:2.19.1:test failed: The forked VM terminated without properly saying goodbye. VM crash or System.exit called? + + +

+ This typically means that your build is running out of memory. HAPI's unit tests execute by + default in multiple threads (the thread count is determined by the number of CPU cores available) + so in an environment with lots of cores but not enough RAM, you may run out. If you are getting + this error, try executing the build with the following arguments: +

+ 
mvn -P ALLMODULES,NOPARALLEL install
+

+ See Hacking HAPI FHIR for more information on + the build process. +

+ + +