Merge branch 'master' into windows-fixes-2

hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/param/DateParam.java

@@ -255,7 +255,15 @@ public class DateParam extends BaseParamWithPrefix<DateParam> implements /*IQuer
 		return b.build();
 	}
 
-	public class DateParamDateTimeHolder extends BaseDateTimeDt {
+	public static class DateParamDateTimeHolder extends BaseDateTimeDt {
+
+		/**
+		 * Constructor
+		 */
+		public DateParamDateTimeHolder() {
+			super();
+		}
+
 		@Override
 		protected TemporalPrecisionEnum getDefaultPrecisionForDatatype() {
 			return TemporalPrecisionEnum.SECOND;

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/SearchBuilder.java

@@ -80,7 +80,6 @@ import org.hl7.fhir.instance.model.api.IIdType;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
-import org.thymeleaf.util.ListUtils;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -385,7 +384,8 @@ public class SearchBuilder implements ISearchBuilder {
 
 		List<Predicate> codePredicates = new ArrayList<>();
 
-		for (IQueryParameterType nextOr : theList) {
+		for (int orIdx = 0; orIdx < theList.size(); orIdx++) {
+			IQueryParameterType nextOr = theList.get(orIdx);
 
 			if (nextOr instanceof ReferenceParam) {
 				ReferenceParam ref = (ReferenceParam) nextOr;
@@ -496,6 +496,8 @@ public class SearchBuilder implements ISearchBuilder {
 
 					boolean foundChainMatch = false;
 
+					for (Class<? extends IBaseResource> nextType : resourceTypes) {
+
 						String chain = ref.getChain();
 						String remainingChain = null;
 						int chainDotIndex = chain.indexOf('.');
@@ -504,7 +506,6 @@ public class SearchBuilder implements ISearchBuilder {
 							chain = chain.substring(0, chainDotIndex);
 						}
 
-					for (Class<? extends IBaseResource> nextType : resourceTypes) {
 						RuntimeResourceDefinition typeDef = myContext.getResourceDefinition(nextType);
 						String subResourceName = typeDef.getName();
 
@@ -531,37 +532,29 @@ public class SearchBuilder implements ISearchBuilder {
 							}
 						}
 
-						IQueryParameterType chainValue;
+						ArrayList<IQueryParameterType> orValues = Lists.newArrayList();
-						if (remainingChain != null) {
+
-							if (param == null || param.getParamType() != RestSearchParameterTypeEnum.REFERENCE) {
+						for (IQueryParameterType next : theList) {
-								ourLog.debug("Type {} parameter {} is not a reference, can not chain {}", nextType.getSimpleName(), chain, remainingChain);
+							String nextValue = next.getValueAsQueryToken(myContext);
+							IQueryParameterType chainValue = mapReferenceChainToRawParamType(remainingChain, param, theParamName, qualifier, nextType, chain, isMeta, nextValue);
+							if (chainValue == null) {
 								continue;
 							}
-
-							chainValue = new ReferenceParam();
-							chainValue.setValueAsQueryToken(myContext, theParamName, qualifier, resourceId);
-							((ReferenceParam) chainValue).setChain(remainingChain);
-						} else if (isMeta) {
-							IQueryParameterType type = myMatchUrlService.newInstanceType(chain);
-							type.setValueAsQueryToken(myContext, theParamName, qualifier, resourceId);
-							chainValue = type;
-						} else {
-							chainValue = toParameterType(param, qualifier, resourceId);
-						}
-
 							foundChainMatch = true;
+							orValues.add(chainValue);
+						}
 
 						Subquery<Long> subQ = myResourceTableQuery.subquery(Long.class);
 						Root<ResourceTable> subQfrom = subQ.from(ResourceTable.class);
 						subQ.select(subQfrom.get("myId").as(Long.class));
 
 						List<List<? extends IQueryParameterType>> andOrParams = new ArrayList<>();
-						andOrParams.add(Collections.singletonList(chainValue));
+						andOrParams.add(orValues);
 
 						/*
 						 * We're doing a chain call, so push the current query root
 						 * and predicate list down and put new ones at the top of the
-						 * stack and run a subuery
+						 * stack and run a subquery
 						 */
 						Root<ResourceTable> stackRoot = myResourceTableRoot;
 						ArrayList<Predicate> stackPredicates = myPredicates;
@@ -573,9 +566,11 @@ public class SearchBuilder implements ISearchBuilder {
 						// Create the subquery predicates
 						myPredicates.add(myBuilder.equal(myResourceTableRoot.get("myResourceType"), subResourceName));
 						myPredicates.add(myBuilder.isNull(myResourceTableRoot.get("myDeleted")));
-						searchForIdsWithAndOr(subResourceName, chain, andOrParams);
 
+						if (foundChainMatch) {
+							searchForIdsWithAndOr(subResourceName, chain, andOrParams);
 							subQ.where(toArray(myPredicates));
+						}
 
 						/*
 						 * Pop the old query root and predicate list back
@@ -593,6 +588,10 @@ public class SearchBuilder implements ISearchBuilder {
 					if (!foundChainMatch) {
 						throw new InvalidRequestException(myContext.getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "invalidParameterChain", theParamName + '.' + ref.getChain()));
 					}
+
+					myPredicates.add(myBuilder.or(toArray(codePredicates)));
+					return;
+
 				}
 
 			} else {
@@ -604,6 +603,28 @@ public class SearchBuilder implements ISearchBuilder {
 		myPredicates.add(myBuilder.or(toArray(codePredicates)));
 	}
 
+	private IQueryParameterType mapReferenceChainToRawParamType(String remainingChain, RuntimeSearchParam param, String theParamName, String qualifier, Class<? extends IBaseResource> nextType, String chain, boolean isMeta, String resourceId) {
+		IQueryParameterType chainValue;
+		if (remainingChain != null) {
+			if (param == null || param.getParamType() != RestSearchParameterTypeEnum.REFERENCE) {
+				ourLog.debug("Type {} parameter {} is not a reference, can not chain {}", nextType.getSimpleName(), chain, remainingChain);
+				return null;
+			}
+
+			chainValue = new ReferenceParam();
+			chainValue.setValueAsQueryToken(myContext, theParamName, qualifier, resourceId);
+			((ReferenceParam) chainValue).setChain(remainingChain);
+		} else if (isMeta) {
+			IQueryParameterType type = myMatchUrlService.newInstanceType(chain);
+			type.setValueAsQueryToken(myContext, theParamName, qualifier, resourceId);
+			chainValue = type;
+		} else {
+			chainValue = toParameterType(param, qualifier, resourceId);
+		}
+
+		return chainValue;
+	}
+
 	private void addPredicateResourceId(List<List<? extends IQueryParameterType>> theValues) {
 		for (List<? extends IQueryParameterType> nextValue : theValues) {
 			Set<Long> orPids = new HashSet<>();
@@ -794,24 +815,27 @@ public class SearchBuilder implements ISearchBuilder {
 
 	private void addPredicateToken(String theResourceName, String theParamName, List<? extends IQueryParameterType> theList) {
 
-		Join<ResourceTable, ResourceIndexedSearchParamToken> join = createOrReuseJoin(JoinEnum.TOKEN, theParamName);
-
 		if (theList.get(0).getMissing() != null) {
+			Join<ResourceTable, ResourceIndexedSearchParamToken> join = createOrReuseJoin(JoinEnum.TOKEN, theParamName);
 			addPredicateParamMissing(theResourceName, theParamName, theList.get(0).getMissing(), join);
 			return;
 		}
 
 		List<Predicate> codePredicates = new ArrayList<>();
+		Join<ResourceTable, ResourceIndexedSearchParamToken> join = null;
 		for (IQueryParameterType nextOr : theList) {
 
 			if (nextOr instanceof TokenParam) {
 				TokenParam id = (TokenParam) nextOr;
 				if (id.isText()) {
 					addPredicateString(theResourceName, theParamName, theList);
-					continue;
+					break;
 				}
 			}
 
+			if (join == null) {
+				join = createOrReuseJoin(JoinEnum.TOKEN, theParamName);
+			}
 			Predicate singleCode = createPredicateToken(nextOr, theResourceName, theParamName, myBuilder, join);
 			codePredicates.add(singleCode);
 		}
@@ -972,8 +996,9 @@ public class SearchBuilder implements ISearchBuilder {
 
 	@SuppressWarnings("unchecked")
 	private <T> Join<ResourceTable, T> createOrReuseJoin(JoinEnum theType, String theSearchParameterName) {
+		JoinKey key = new JoinKey(theSearchParameterName, theType);
+		return (Join<ResourceTable, T>) myIndexJoins.computeIfAbsent(key, k -> {
 			Join<ResourceTable, ResourceIndexedSearchParamDate> join = null;
-
 			switch (theType) {
 				case DATE:
 					join = myResourceTableRoot.join("myParamsDate", JoinType.LEFT);
@@ -997,13 +1022,8 @@ public class SearchBuilder implements ISearchBuilder {
 					join = myResourceTableRoot.join("myParamsToken", JoinType.LEFT);
 					break;
 			}
-
+			return join;
-		JoinKey key = new JoinKey(theSearchParameterName, theType);
+		});
-		if (!myIndexJoins.containsKey(key)) {
-			myIndexJoins.put(key, join);
-		}
-
-		return (Join<ResourceTable, T>) join;
 	}
 
 	private Predicate createPredicateDate(IQueryParameterType theParam, String theResourceName, String theParamName, CriteriaBuilder theBuilder, From<?, ResourceIndexedSearchParamDate> theFrom) {

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/CaptureQueriesListener.java

@@ -0,0 +1,92 @@
+package ca.uhn.fhir.jpa.config;
+
+import net.ttddyy.dsproxy.ExecutionInfo;
+import net.ttddyy.dsproxy.QueryInfo;
+import net.ttddyy.dsproxy.proxy.ParameterSetOperation;
+import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder;
+import org.hibernate.engine.jdbc.internal.BasicFormatterImpl;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.stream.Collectors;
+
+public class CaptureQueriesListener implements ProxyDataSourceBuilder.SingleQueryExecution {
+
+	private static final LinkedList<Query> LAST_N_QUERIES = new LinkedList<>();
+
+	@Override
+	public void execute(ExecutionInfo execInfo, List<QueryInfo> queryInfoList) {
+		synchronized (LAST_N_QUERIES) {
+			for (QueryInfo next : queryInfoList) {
+				String sql = next.getQuery();
+				List<String> params;
+				if (next.getParametersList().size() > 0 && next.getParametersList().get(0).size() > 0) {
+					List<ParameterSetOperation> values = next
+						.getParametersList()
+						.get(0);
+					params = values.stream()
+						.map(t -> t.getArgs()[1])
+						.map(t -> t != null ? t.toString() : "NULL")
+						.collect(Collectors.toList());
+				} else {
+					params = new ArrayList<>();
+				}
+				LAST_N_QUERIES.add(0, new Query(sql, params));
+			}
+			while (LAST_N_QUERIES.size() > 100) {
+				LAST_N_QUERIES.removeLast();
+			}
+		}
+	}
+
+	public static class Query {
+		private final String myThreadName = Thread.currentThread().getName();
+		private final String mySql;
+		private final List<String> myParams;
+
+		Query(String theSql, List<String> theParams) {
+			mySql = theSql;
+			myParams = Collections.unmodifiableList(theParams);
+		}
+
+		public String getThreadName() {
+			return myThreadName;
+		}
+
+		public String getSql(boolean theInlineParams, boolean theFormat) {
+			String retVal = mySql;
+			if (theFormat) {
+				retVal = new BasicFormatterImpl().format(retVal);
+			}
+
+			if (theInlineParams) {
+				List<String> nextParams = new ArrayList<>(myParams);
+				while (retVal.contains("?") && nextParams.size() > 0) {
+					int idx = retVal.indexOf("?");
+					retVal = retVal.substring(0, idx) + nextParams.remove(0) + retVal.substring(idx + 1);
+				}
+			}
+
+			return retVal;
+
+		}
+
+	}
+
+	public static void clear() {
+		synchronized (LAST_N_QUERIES) {
+			LAST_N_QUERIES.clear();
+		}
+	}
+
+	/**
+	 * Index 0 is newest!
+	 */
+	public static ArrayList<Query> getLastNQueries() {
+		synchronized (LAST_N_QUERIES) {
+			return new ArrayList<>(LAST_N_QUERIES);
+		}
+	}
+}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java

@@ -100,6 +100,7 @@ public class TestR4Config extends BaseJavaConfigR4 {
 //			.logSlowQueryBySlf4j(10, TimeUnit.SECONDS)
 //			.countQuery(new ThreadQueryCountHolder())
 			.beforeQuery(new BlockLargeNumbersOfParamsListener())
+			.afterQuery(new CaptureQueriesListener())
 			.countQuery(singleQueryCountHolder())
 			.build();
 

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/BaseJpaTest.java

@@ -1,6 +1,7 @@
 package ca.uhn.fhir.jpa.dao;
 
 import ca.uhn.fhir.context.FhirContext;
+import ca.uhn.fhir.jpa.config.CaptureQueriesListener;
 import ca.uhn.fhir.jpa.entity.TermConcept;
 import ca.uhn.fhir.jpa.model.interceptor.api.IInterceptorRegistry;
 import ca.uhn.fhir.jpa.model.interceptor.api.Pointcut;
@@ -94,6 +95,7 @@ public abstract class BaseJpaTest {
 	@After
 	public void afterPerformCleanup() {
 		BaseHapiFhirResourceDao.setDisableIncrementOnUpdateForUnitTest(false);
+		CaptureQueriesListener.clear();
 	}
 
 	@After

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4SearchNoFtTest.java

@@ -1,9 +1,10 @@
 package ca.uhn.fhir.jpa.dao.r4;
 
+import ca.uhn.fhir.jpa.config.CaptureQueriesListener;
 import ca.uhn.fhir.jpa.dao.DaoConfig;
+import ca.uhn.fhir.jpa.model.entity.*;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
 import ca.uhn.fhir.jpa.searchparam.SearchParameterMap.EverythingModeEnum;
-import ca.uhn.fhir.jpa.model.entity.*;
 import ca.uhn.fhir.jpa.util.TestUtil;
 import ca.uhn.fhir.model.api.Include;
 import ca.uhn.fhir.model.api.TemporalPrecisionEnum;
@@ -13,6 +14,7 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider;
 import ca.uhn.fhir.rest.param.*;
 import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
 import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException;
+import com.google.common.collect.Lists;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.hl7.fhir.instance.model.api.IAnyResource;
@@ -39,6 +41,7 @@ import java.io.IOException;
 import java.math.BigDecimal;
 import java.nio.charset.StandardCharsets;
 import java.util.*;
+import java.util.stream.Collectors;
 
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
@@ -53,6 +56,7 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
 		myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis());
 		myDaoConfig.setFetchSizeDefaultMaximum(new DaoConfig().getFetchSizeDefaultMaximum());
 		myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches());
+		myDaoConfig.setSearchPreFetchThresholds(new DaoConfig().getSearchPreFetchThresholds());
 	}
 
 	@Before
@@ -1140,6 +1144,40 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
 
 	}
 
+	/**
+	 * See #1174
+	 */
+	@Test
+	public void testSearchDateInSavedSearch() {
+		for (int i = 1; i <= 9; i++) {
+			Patient p1 = new Patient();
+			p1.getBirthDateElement().setValueAsString("1980-01-0" + i);
+			String id1 = myPatientDao.create(p1).getId().toUnqualifiedVersionless().getValue();
+		}
+
+		myDaoConfig.setSearchPreFetchThresholds(Lists.newArrayList(3, 6, 10));
+
+		{
+			// Don't load synchronous
+			SearchParameterMap map = new SearchParameterMap();
+			map.setLastUpdated(new DateRangeParam().setUpperBound(new DateParam(ParamPrefixEnum.LESSTHAN, "2022-01-01")));
+			IBundleProvider found = myPatientDao.search(map);
+			Set<String> dates = new HashSet<>();
+			for (int i = 0; i < 9; i++) {
+				Patient nextResource = (Patient) found.getResources(i, i + 1).get(0);
+				dates.add(nextResource.getBirthDateElement().getValueAsString());
+			}
+
+			assertThat(dates, hasItems(
+				"1980-01-01",
+				"1980-01-09"
+			));
+
+			assertFalse(map.isLoadSynchronous());
+			assertNull(map.getLoadSynchronousUpTo());
+		}
+	}
+
 	/**
 	 * #222
 	 */
@@ -2160,6 +2198,51 @@ public class FhirResourceDaoR4SearchNoFtTest extends BaseJpaR4Test {
 
 	}
 
+	@Test
+	public void testSearchLinkToken() {
+		// /fhirapi/MedicationRequest?category=community&identifier=urn:oid:2.16.840.1.113883.3.7418.12.3%7C&intent=order&medication.code:text=calcitriol,hectorol,Zemplar,rocaltrol,vectical,vitamin%20D,doxercalciferol,paricalcitol&status=active,completed
+
+		Medication m = new Medication();
+		m.getCode().setText("valueb");
+		myMedicationDao.create(m);
+
+		MedicationRequest mr = new MedicationRequest();
+		mr.addCategory().addCoding().setCode("community");
+		mr.addIdentifier().setSystem("urn:oid:2.16.840.1.113883.3.7418.12.3").setValue("1");
+		mr.setIntent(MedicationRequest.MedicationRequestIntent.ORDER);
+		mr.setMedication(new Reference(m.getId()));
+		myMedicationRequestDao.create(mr);
+
+		SearchParameterMap sp = new SearchParameterMap();
+		sp.setLoadSynchronous(true);
+		sp.add("category", new TokenParam("community"));
+		sp.add("identifier", new TokenParam("urn:oid:2.16.840.1.113883.3.7418.12.3", "1"));
+		sp.add("intent", new TokenParam("order"));
+		ReferenceParam param1 = new ReferenceParam("valuea").setChain("code:text");
+		ReferenceParam param2 = new ReferenceParam("valueb").setChain("code:text");
+		ReferenceParam param3 = new ReferenceParam("valuec").setChain("code:text");
+		sp.add("medication", new ReferenceOrListParam().addOr(param1).addOr(param2).addOr(param3));
+
+		IBundleProvider retrieved = myMedicationRequestDao.search(sp);
+		assertEquals(1, retrieved.size().intValue());
+
+		List<String> queries = CaptureQueriesListener
+			.getLastNQueries()
+			.stream()
+			.filter(t -> t.getThreadName().equals("main"))
+			.filter(t -> t.getSql(false, false).toLowerCase().contains("select"))
+			.filter(t -> t.getSql(false, false).toLowerCase().contains("token"))
+			.map(t -> t.getSql(true, true))
+			.collect(Collectors.toList());
+
+		ourLog.info("Queries:\n  {}", queries.stream().findFirst());
+
+		String searchQuery = queries.get(0);
+		assertEquals(searchQuery, 3, StringUtils.countMatches(searchQuery.toUpperCase(), "HFJ_SPIDX_TOKEN"));
+		assertEquals(searchQuery, 5, StringUtils.countMatches(searchQuery.toUpperCase(), "LEFT OUTER JOIN"));
+	}
+
+
 	@Test
 	public void testSearchTokenParam() {
 		Patient patient = new Patient();

pom.xml

@@ -534,7 +534,7 @@
 		<jetty_version>9.4.14.v20181114</jetty_version>
 		<jsr305_version>3.0.2</jsr305_version>
 		<!--<hibernate_version>5.2.10.Final</hibernate_version>-->
-		<hibernate_version>5.4.0.Final</hibernate_version>
+		<hibernate_version>5.4.1.Final</hibernate_version>
 		<!-- Update lucene version when you update hibernate-search version -->
 		<hibernate_search_version>5.11.0.Final</hibernate_search_version>
 		<lucene_version>5.5.5</lucene_version>

src/changes/changes.xml

@@ -333,6 +333,17 @@
 				whether a call out to the database may be required.  I say "may" because subscription matches fail fast
 				so a negative match may be performed in-memory, but a positive match will require a database call.
 			</action>
+			<action type="fix">
+				When performing a JPA search with a chained :text modifier
+				(e.g. MedicationStatement?medication.code:text=aspirin,tylenol) a series
+				of unneccesary joins were introduced to the generated SQL query, harming
+				performance. This has been fixed.
+			</action>
+			<action type="fix">
+				A serialization error when performing some searches in the JPA server
+				using data parameters has been fixed. Thanks to GitHub user
+				@PickOneFish for reporting!
+			</action>
 		</release>
 		<release version="3.6.0" date="2018-11-12" description="Food">
 			<action type="add">