Fix #863 - Allow custom search parameters to use multiple bases

This commit is contained in:
James Agnew 2018-02-24 22:10:55 -05:00
parent 1a9522cfdd
commit 136455f312
7 changed files with 226 additions and 80 deletions

View File

@ -9,9 +9,9 @@ package ca.uhn.fhir.jpa.dao;
* 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.
@ -83,7 +83,7 @@ public class SearchBuilder implements ISearchBuilder {
private static final List<Long> EMPTY_LONG_LIST = Collections.unmodifiableList(new ArrayList<Long>());
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchBuilder.class);
private static Long NO_MORE = Long.valueOf(-1);
private static Long NO_MORE = -1L;
private static HandlerTypeEnum ourLastHandlerMechanismForUnitTest;
private List<Long> myAlsoIncludePids;
private CriteriaBuilder myBuilder;
@ -331,10 +331,9 @@ public class SearchBuilder implements ISearchBuilder {
List<Predicate> codePredicates = new ArrayList<Predicate>();
for (IQueryParameterType nextOr : theList) {
IQueryParameterType params = nextOr;
if (params instanceof ReferenceParam) {
ReferenceParam ref = (ReferenceParam) params;
if (nextOr instanceof ReferenceParam) {
ReferenceParam ref = (ReferenceParam) nextOr;
if (isBlank(ref.getChain())) {
IIdType dt = new IdDt(ref.getBaseUrl(), ref.getResourceType(), ref.getIdPart(), null);
@ -471,7 +470,7 @@ public class SearchBuilder implements ISearchBuilder {
IQueryParameterType chainValue;
if (remainingChain != null) {
if (param == null || param.getParamType() != RestSearchParameterTypeEnum.REFERENCE) {
ourLog.debug("Type {} parameter {} is not a reference, can not chain {}", new Object[]{nextType.getSimpleName(), chain, remainingChain});
ourLog.debug("Type {} parameter {} is not a reference, can not chain {}", new Object[] {nextType.getSimpleName(), chain, remainingChain});
continue;
}
@ -533,7 +532,7 @@ public class SearchBuilder implements ISearchBuilder {
}
} else {
throw new IllegalArgumentException("Invalid token type (expecting ReferenceParam): " + params.getClass());
throw new IllegalArgumentException("Invalid token type (expecting ReferenceParam): " + nextOr.getClass());
}
}
@ -1475,37 +1474,37 @@ public class SearchBuilder implements ISearchBuilder {
switch (param.getParamType()) {
case STRING:
joinAttrName = "myParamsString";
sortAttrName = new String[]{"myValueExact"};
sortAttrName = new String[] {"myValueExact"};
joinType = JoinEnum.STRING;
break;
case DATE:
joinAttrName = "myParamsDate";
sortAttrName = new String[]{"myValueLow"};
sortAttrName = new String[] {"myValueLow"};
joinType = JoinEnum.DATE;
break;
case REFERENCE:
joinAttrName = "myResourceLinks";
sortAttrName = new String[]{"myTargetResourcePid"};
sortAttrName = new String[] {"myTargetResourcePid"};
joinType = JoinEnum.REFERENCE;
break;
case TOKEN:
joinAttrName = "myParamsToken";
sortAttrName = new String[]{"mySystem", "myValue"};
sortAttrName = new String[] {"mySystem", "myValue"};
joinType = JoinEnum.TOKEN;
break;
case NUMBER:
joinAttrName = "myParamsNumber";
sortAttrName = new String[]{"myValue"};
sortAttrName = new String[] {"myValue"};
joinType = JoinEnum.NUMBER;
break;
case URI:
joinAttrName = "myParamsUri";
sortAttrName = new String[]{"myUri"};
sortAttrName = new String[] {"myUri"};
joinType = JoinEnum.URI;
break;
case QUANTITY:
joinAttrName = "myParamsQuantity";
sortAttrName = new String[]{"myValue"};
sortAttrName = new String[] {"myValue"};
joinType = JoinEnum.QUANTITY;
break;
default:
@ -1780,7 +1779,7 @@ public class SearchBuilder implements ISearchBuilder {
nextRoundMatches = pidsToInclude;
} while (includes.size() > 0 && nextRoundMatches.size() > 0 && addedSomeThisRound);
ourLog.info("Loaded {} {} in {} rounds and {} ms", new Object[]{allAdded.size(), theReverseMode ? "_revincludes" : "_includes", roundCounts, w.getMillisAndRestart()});
ourLog.info("Loaded {} {} in {} rounds and {} ms", new Object[] {allAdded.size(), theReverseMode ? "_revincludes" : "_includes", roundCounts, w.getMillisAndRestart()});
return allAdded;
}
@ -2006,8 +2005,17 @@ public class SearchBuilder implements ISearchBuilder {
RuntimeResourceDefinition resourceDef = theContext.getResourceDefinition(theResourceType);
RuntimeSearchParam param = theCallingDao.getSearchParamByName(resourceDef, theParamName);
List<String> path = param.getPathsSplit();
Predicate type = theFrom.get("mySourcePath").in(path);
return type;
/*
* SearchParameters can declare paths on multiple resources
* types. Here we only want the ones that actually apply.
*/
for (Iterator<String> iter = path.iterator(); iter.hasNext(); ) {
if (!iter.next().startsWith(theResourceType + ".")) {
iter.remove();
}
}
return theFrom.get("mySourcePath").in(path);
}
private static List<Long> filterResourceIdsByLastUpdated(EntityManager theEntityManager, final DateRangeParam theLastUpdated, Collection<Long> thePids) {

View File

@ -21,6 +21,7 @@ package ca.uhn.fhir.jpa.dao.dstu3;
*/
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.apache.commons.lang3.StringUtils.trim;
import java.math.BigDecimal;
import java.util.*;
@ -478,8 +479,8 @@ public class SearchParamExtractorDstu3 extends BaseSearchParamExtractor implemen
multiType = true;
}
List<String> systems = new ArrayList<String>();
List<String> codes = new ArrayList<String>();
List<String> systems = new ArrayList<>();
List<String> codes = new ArrayList<>();
// String needContactPointSystem = null;
// if (nextPath.contains(".where(system='phone')")) {
@ -693,11 +694,11 @@ public class SearchParamExtractorDstu3 extends BaseSearchParamExtractor implemen
IWorkerContext worker = new org.hl7.fhir.dstu3.hapi.ctx.HapiWorkerContext(getContext(), myValidationSupport);
FHIRPathEngine fp = new FHIRPathEngine(worker);
List<Object> values = new ArrayList<Object>();
List<Object> values = new ArrayList<>();
try {
String[] nextPathsSplit = SPLIT.split(thePaths);
for (String nextPath : nextPathsSplit) {
List<Base> allValues = fp.evaluate((Base) theResource, nextPath);
List<Base> allValues = fp.evaluate((Base) theResource, trim(nextPath));
if (allValues.isEmpty() == false) {
values.addAll(allValues);
}

View File

@ -40,9 +40,6 @@ import static org.apache.commons.lang3.StringUtils.isBlank;
public class FhirResourceDaoSearchParameterR4 extends FhirResourceDaoR4<SearchParameter> implements IFhirResourceDaoSearchParameter<SearchParameter> {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoSearchParameterR4.class);
@Autowired
private IFhirSystemDao<Bundle, Meta> mySystemDao;
@ -130,7 +127,6 @@ public class FhirResourceDaoSearchParameterR4 extends FhirResourceDaoR4<SearchPa
theExpression = theExpression.trim();
String[] expressionSplit = BaseSearchParamExtractor.SPLIT.split(theExpression);
String allResourceName = null;
for (String nextPath : expressionSplit) {
nextPath = nextPath.trim();
@ -146,14 +142,6 @@ public class FhirResourceDaoSearchParameterR4 extends FhirResourceDaoR4<SearchPa
throw new UnprocessableEntityException("Invalid SearchParameter.expression value \"" + nextPath + "\": " + e.getMessage());
}
if (allResourceName == null) {
allResourceName = resourceName;
} else {
if (!allResourceName.equals(resourceName)) {
throw new UnprocessableEntityException("Invalid SearchParameter.expression value \"" + nextPath + "\". All paths in a single SearchParameter must match the same resource type");
}
}
}
} // if have expression

View File

@ -1,17 +1,5 @@
package ca.uhn.fhir.jpa.dao.dstu3;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import java.util.List;
import org.hl7.fhir.dstu3.model.*;
import org.hl7.fhir.dstu3.model.Appointment.AppointmentStatus;
import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender;
import org.hl7.fhir.exceptions.FHIRException;
import org.hl7.fhir.instance.model.api.IIdType;
import org.junit.*;
import ca.uhn.fhir.jpa.dao.SearchParameterMap;
import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
@ -19,6 +7,19 @@ import ca.uhn.fhir.rest.param.*;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import ca.uhn.fhir.util.TestUtil;
import org.hl7.fhir.dstu3.model.*;
import org.hl7.fhir.dstu3.model.Appointment.AppointmentStatus;
import org.hl7.fhir.dstu3.model.Enumerations.AdministrativeGender;
import org.hl7.fhir.exceptions.FHIRException;
import org.hl7.fhir.instance.model.api.IIdType;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.Test;
import java.util.List;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu3Test {
private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoDstu3SearchCustomSearchParamTest.class);
@ -228,6 +229,76 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
}
/**
* See #863
*/
@Test
public void testParamWithMultipleBasesReference() {
SearchParameter sp = new SearchParameter();
sp.setUrl("http://clinicalcloud.solutions/fhir/SearchParameter/request-reason");
sp.setName("reason");
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
sp.setCode("reason");
sp.addBase("MedicationRequest");
sp.addBase("ProcedureRequest");
sp.setType(Enumerations.SearchParamType.REFERENCE);
sp.setExpression("MedicationRequest.reasonReference | ProcedureRequest.reasonReference");
sp.addTarget("Condition");
sp.addTarget("Observation");
mySearchParameterDao.create(sp);
mySearchParamRegsitry.forceRefresh();
Condition condition = new Condition();
condition.getCode().setText("A condition");
String conditionId = myConditionDao.create(condition).getId().toUnqualifiedVersionless().getValue();
MedicationRequest mr = new MedicationRequest();
mr.addReasonReference().setReference(conditionId);
String mrId = myMedicationRequestDao.create(mr).getId().toUnqualifiedVersionless().getValue();
ProcedureRequest pr = new ProcedureRequest();
pr.addReasonReference().setReference(conditionId);
myProcedureRequestDao.create(pr);
SearchParameterMap map = new SearchParameterMap();
map.setLoadSynchronous(true);
map.add("reason", new ReferenceParam(conditionId));
List<String> results = toUnqualifiedVersionlessIdValues(myMedicationRequestDao.search(map));
assertThat(results, contains(mrId));
}
/**
* See #863
*/
@Test
public void testParamWithMultipleBasesToken() {
SearchParameter sp = new SearchParameter();
sp.setUrl("http://clinicalcloud.solutions/fhir/SearchParameter/request-reason");
sp.setName("reason");
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
sp.setCode("reason");
sp.addBase("MedicationRequest");
sp.addBase("ProcedureRequest");
sp.setType(Enumerations.SearchParamType.TOKEN);
sp.setExpression("MedicationRequest.reasonCode | ProcedureRequest.reasonCode");
mySearchParameterDao.create(sp);
mySearchParamRegsitry.forceRefresh();
MedicationRequest mr = new MedicationRequest();
mr.addReasonCode().addCoding().setSystem("foo").setCode("bar");
String mrId = myMedicationRequestDao.create(mr).getId().toUnqualifiedVersionless().getValue();
ProcedureRequest pr = new ProcedureRequest();
pr.addReasonCode().addCoding().setSystem("foo").setCode("bar");
myProcedureRequestDao.create(pr);
SearchParameterMap map = new SearchParameterMap();
map.setLoadSynchronous(true);
map.add("reason", new TokenParam("foo", "bar"));
List<String> results = toUnqualifiedVersionlessIdValues(myMedicationRequestDao.search(map));
assertThat(results, contains(mrId));
}
@Test
public void testSearchForExtensionReferenceWithNonMatchingTarget() {
SearchParameter siblingSp = new SearchParameter();
@ -271,7 +342,7 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
assertThat(foundResources, empty());
}
@Test
public void testSearchForExtensionReferenceWithTarget() {
SearchParameter siblingSp = new SearchParameter();
@ -414,7 +485,7 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
assertThat(foundResources, contains(p1id.getValue()));
}
@Test
public void testSearchForExtensionTwoDeepCodeableConcept() {
SearchParameter siblingSp = new SearchParameter();
@ -436,9 +507,9 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.addExtension()
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new CodeableConcept().addCoding(new Coding().setSystem("foo").setCode("bar")));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new CodeableConcept().addCoding(new Coding().setSystem("foo").setCode("bar")));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
@ -474,9 +545,9 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.addExtension()
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new Coding().setSystem("foo").setCode("bar"));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new Coding().setSystem("foo").setCode("bar"));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
@ -516,9 +587,9 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new DateType("2012-01-02"));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new DateType("2012-01-02"));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
@ -553,16 +624,16 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.addExtension()
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new DecimalType("2.1"));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new DecimalType("2.1"));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
SearchParameterMap map;
IBundleProvider results;
List<String> foundResources;
map = new SearchParameterMap();
map.add("foobar", new NumberParam("2.1"));
results = myPatientDao.search(map);
@ -590,9 +661,9 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.addExtension()
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new IntegerType(5));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new IntegerType(5));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
@ -633,9 +704,9 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new Reference(aptId.getValue()));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new Reference(aptId.getValue()));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
@ -675,9 +746,9 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new Reference(aptId.getValue()));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new Reference(aptId.getValue()));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
@ -718,9 +789,9 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new Reference(aptId.getValue()));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new Reference(aptId.getValue()));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
@ -755,9 +826,9 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
.addExtension()
.setUrl("http://acme.org/foo");
extParent
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new StringType("HELLOHELLO"));
.addExtension()
.setUrl("http://acme.org/bar")
.setValue(new StringType("HELLOHELLO"));
IIdType p2id = myPatientDao.create(patient).getId().toUnqualifiedVersionless();
@ -861,7 +932,7 @@ public class FhirResourceDaoDstu3SearchCustomSearchParamTest extends BaseJpaDstu
assertEquals("Unknown search parameter foo for resource type Patient", e.getMessage());
}
}
@Test
public void testSearchWithCustomParamDraft() {

View File

@ -334,6 +334,77 @@ public class FhirResourceDaoR4SearchCustomSearchParamTest extends BaseJpaR4Test
}), empty());
}
/**
* See #863
*/
@Test
public void testParamWithMultipleBases() {
SearchParameter sp = new SearchParameter();
sp.setUrl("http://clinicalcloud.solutions/fhir/SearchParameter/request-reason");
sp.setName("reason");
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
sp.setCode("reason");
sp.addBase("MedicationRequest");
sp.addBase("ServiceRequest");
sp.setType(Enumerations.SearchParamType.REFERENCE);
sp.setExpression("MedicationRequest.reasonReference | ServiceRequest.reasonReference");
sp.addTarget("Condition");
sp.addTarget("Observation");
mySearchParameterDao.create(sp);
mySearchParamRegsitry.forceRefresh();
Condition condition = new Condition();
condition.getCode().setText("A condition");
String conditionId = myConditionDao.create(condition).getId().toUnqualifiedVersionless().getValue();
MedicationRequest mr = new MedicationRequest();
mr.addReasonReference().setReference(conditionId);
String mrId = myMedicationRequestDao.create(mr).getId().toUnqualifiedVersionless().getValue();
ServiceRequest pr = new ServiceRequest();
pr.addReasonReference().setReference(conditionId);
myServiceRequestDao.create(pr);
SearchParameterMap map = new SearchParameterMap();
map.setLoadSynchronous(true);
map.add("reason", new ReferenceParam(conditionId));
List<String> results = toUnqualifiedVersionlessIdValues(myMedicationRequestDao.search(map));
assertThat(results, contains(mrId));
}
/**
* See #863
*/
@Test
public void testParamWithMultipleBasesToken() {
SearchParameter sp = new SearchParameter();
sp.setUrl("http://clinicalcloud.solutions/fhir/SearchParameter/request-reason");
sp.setName("reason");
sp.setStatus(Enumerations.PublicationStatus.ACTIVE);
sp.setCode("reason");
sp.addBase("MedicationRequest");
sp.addBase("ServiceRequest");
sp.setType(Enumerations.SearchParamType.TOKEN);
sp.setExpression("MedicationRequest.reasonCode | ServiceRequest.reasonCode");
mySearchParameterDao.create(sp);
mySearchParamRegsitry.forceRefresh();
MedicationRequest mr = new MedicationRequest();
mr.addReasonCode().addCoding().setSystem("foo").setCode("bar");
String mrId = myMedicationRequestDao.create(mr).getId().toUnqualifiedVersionless().getValue();
ServiceRequest pr = new ServiceRequest();
pr.addReasonCode().addCoding().setSystem("foo").setCode("bar");
myServiceRequestDao.create(pr);
SearchParameterMap map = new SearchParameterMap();
map.setLoadSynchronous(true);
map.add("reason", new TokenParam("foo", "bar"));
List<String> results = toUnqualifiedVersionlessIdValues(myMedicationRequestDao.search(map));
assertThat(results, contains(mrId));
}
@Test
public void testSearchForExtensionReferenceWithNonMatchingTarget() {
SearchParameter siblingSp = new SearchParameter();

View File

@ -1118,8 +1118,8 @@
<encoding>UTF-8</encoding>
<fork>true</fork>
<meminitial>128m</meminitial>
<maxmem>1600m</maxmem>
<meminitial>500m</meminitial>
<maxmem>2000m</maxmem>
</configuration>
<dependencies>
<dependency>

View File

@ -158,6 +158,13 @@
A number of info level log lines have been reduced to debug level in the JPA server, in
order to reduce contention during heavy loads.
</action>
<action type="fix" issue="863">
JPA server now correctly indexes custom search parameters which
have multiple base resource types. Previously, the indexing could
cause resources of the wrong type to be returned in a search
if a parameter being used also matched that type. Thanks
to Dave Carlson for reporting!
</action>
</release>
<release version="3.2.0" date="2018-01-13">
<action type="add">