honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix 2 issues with has reverse chaining (#4139) 

* Fix 2 issues with chains

* Add changelog

* Cleanup

* Add comment

* Cleanup
This commit is contained in:
James Agnew 2022-10-14 16:46:18 -04:00 committed by GitHub
parent f00f65aae4
commit 26ef32c1dd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 122 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_2_0/4139-reverse-chaining-fixes.yaml Normal file
View File

@ -0,0 +1,6 @@
---
type: fix
issue: 4139
title: "Two issues with reverse chaining (i.e. the `_has` search parameter) have been addressed:
* Searching for a reverse chain with a target search parameter of `_id` did not work correctly, e.g. `Patient?_has:Observation:subject:_id=Patient/123`
* Searching with a combination of a forward chain and a reverse chain did not work correctly if indexing contained resources was enabled, e.g. `Observation?subject._has:Group:member:_id=Group/123`"

45
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/builder/QueryStack.java
View File

@ -128,6 +128,8 @@ import static ca.uhn.fhir.jpa.util.QueryParameterUtils.toAndPredicate;
import static ca.uhn.fhir.jpa.util.QueryParameterUtils.toEqualToOrInPredicate;
import static ca.uhn.fhir.jpa.util.QueryParameterUtils.toOperation;
import static ca.uhn.fhir.jpa.util.QueryParameterUtils.toOrPredicate;
import static ca.uhn.fhir.rest.api.Constants.PARAM_HAS;
import static ca.uhn.fhir.rest.api.Constants.PARAM_ID;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.apache.commons.lang3.StringUtils.split;
@ -256,7 +258,7 @@ public class QueryStack {
TokenPredicateBuilder tokenPredicateBuilder = mySqlBuilder.createTokenPredicateBuilder();
Condition hashIdentityPredicate = tokenPredicateBuilder.createHashIdentityPredicate(theResourceName, theParamName);
addSortCustomJoin(firstPredicateBuilder, tokenPredicateBuilder, hashIdentityPredicate);
mySqlBuilder.addSortString(tokenPredicateBuilder.getColumnSystem(), theAscending);
@ -727,10 +729,16 @@ public class QueryStack {
String qualifier = paramName.substring(4);
for (IQueryParameterType next : nextOrList) {
HasParam nextHasParam = new HasParam();
nextHasParam.setValueAsQueryToken(myFhirContext, Constants.PARAM_HAS, qualifier, next.getValueAsQueryToken(myFhirContext));
nextHasParam.setValueAsQueryToken(myFhirContext, PARAM_HAS, qualifier, next.getValueAsQueryToken(myFhirContext));
orValues.add(nextHasParam);
}
} else if (paramName.equals(PARAM_ID)) {
for (IQueryParameterType next : nextOrList) {
orValues.add(new TokenParam(next.getValueAsQueryToken(myFhirContext)));
}
} else {
//Ensure that the name of the search param
@ -959,7 +967,7 @@ public class QueryStack {
}
public String getPath() { return myPath; }
public String getSearchParameterName() { return mySearchParameterName; }
@Override
@ -1006,7 +1014,6 @@ public class QueryStack {
String targetValue = nextOr.getValueAsQueryToken(myFhirContext);
if (nextOr instanceof ReferenceParam) {
ReferenceParam referenceParam = (ReferenceParam) nextOr;
if (!isReferenceParamValid(referenceParam)) {
throw new InvalidRequestException(Msg.code(2007) +
"The search chain " + theSearchParam.getName() + "." + referenceParam.getChain() +
@ -1538,11 +1545,11 @@ public class QueryStack {
String theSpnamePrefix, RuntimeSearchParam theSearchParam, List<? extends IQueryParameterType> theList,
SearchFilterParser.CompareOperation theOperation, RequestPartitionId theRequestPartitionId, SearchQueryBuilder theSqlBuilder) {
List<IQueryParameterType> tokens = new ArrayList<>();
List<IQueryParameterType> tokens = new ArrayList<>();
boolean paramInverted = false;
TokenParamModifier modifier;
for (IQueryParameterType nextOr : theList) {
if (nextOr instanceof TokenParam) {
if (!((TokenParam) nextOr).isEmpty()) {
@ -1561,8 +1568,8 @@ public class QueryStack {
throw new MethodNotAllowedException(Msg.code(1219) + msg);
}
return createPredicateString(theSourceJoinColumn, theResourceName, theSpnamePrefix, theSearchParam, theList, null, theRequestPartitionId, theSqlBuilder);
}
}
modifier = id.getModifier();
// for :not modifier, create a token and remove the :not modifier
if (modifier == TokenParamModifier.NOT) {
@ -1584,23 +1591,23 @@ public class QueryStack {
String paramName = getParamNameWithPrefix(theSpnamePrefix, theSearchParam.getName());
Condition predicate;
BaseJoiningPredicateBuilder join;
if (paramInverted) {
SearchQueryBuilder sqlBuilder = theSqlBuilder.newChildSqlBuilder();
TokenPredicateBuilder tokenSelector = sqlBuilder.addTokenPredicateBuilder(null);
sqlBuilder.addPredicate(tokenSelector.createPredicateToken(tokens, theResourceName, theSpnamePrefix, theSearchParam, theRequestPartitionId));
SelectQuery sql = sqlBuilder.getSelect();
Expression subSelect = new Subquery(sql);
join = theSqlBuilder.getOrCreateFirstPredicateBuilder();
if (theSourceJoinColumn == null) {
predicate = new InCondition(join.getResourceIdColumn(), subSelect).setNegate(true);
} else {
//-- for the resource link, need join with target_resource_id
predicate = new InCondition(theSourceJoinColumn, subSelect).setNegate(true);
}
} else {
Boolean isMissing = theList.get(0).getMissing();
if (isMissing != null) {
@ -1620,9 +1627,9 @@ public class QueryStack {
TokenPredicateBuilder tokenJoin = createOrReusePredicateBuilder(PredicateBuilderTypeEnum.TOKEN, theSourceJoinColumn, paramName, () -> theSqlBuilder.addTokenPredicateBuilder(theSourceJoinColumn)).getResult();
predicate = tokenJoin.createPredicateToken(tokens, theResourceName, theSpnamePrefix, theSearchParam, theOperation, theRequestPartitionId);
join = tokenJoin;
}
join = tokenJoin;
}
return join.combineWithRequestPartitionIdPredicate(theRequestPartitionId, predicate);
}
@ -1676,7 +1683,7 @@ public class QueryStack {
case IAnyResource.SP_RES_ID:
return createPredicateResourceId(theSourceJoinColumn, theAndOrParams, theResourceName, null, theRequestPartitionId);
case Constants.PARAM_HAS:
case PARAM_HAS:
return createPredicateHas(theSourceJoinColumn, theResourceName, theAndOrParams, theRequest, theRequestPartitionId);
case Constants.PARAM_TAG:
@ -1820,7 +1827,9 @@ public class QueryStack {
nextAnd.stream()
.filter(t -> t instanceof ReferenceParam)
.map(t -> ((ReferenceParam) t).getChain())
.anyMatch(StringUtils::isNotBlank);
.filter(StringUtils::isNotBlank)
// Chains on _has can't be indexed for contained searches - At least not yet. It's not clear to me if we ever want to support this, it would be really hard to do.
.anyMatch(t->!t.startsWith(PARAM_HAS + ":"));
}
public void addPredicateCompositeUnique(String theIndexString, RequestPartitionId theRequestPartitionId) {

82
hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/provider/r4/ResourceProviderR4Test.java
View File

@ -4,6 +4,7 @@ import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.jpa.api.config.DaoConfig;
import ca.uhn.fhir.jpa.dao.data.ISearchDao;
import ca.uhn.fhir.jpa.entity.Search;
import ca.uhn.fhir.jpa.model.entity.ModelConfig;
import ca.uhn.fhir.jpa.model.entity.NormalizedQuantitySearchLevel;
import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
import ca.uhn.fhir.jpa.model.util.JpaConstants;
@ -70,6 +71,7 @@ import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.apache.http.message.BasicNameValuePair;
import org.apache.http.util.EntityUtils;
import org.apache.jena.rdf.model.ModelCon;
import org.hamcrest.Matchers;
import org.hl7.fhir.common.hapi.validation.validator.FhirInstanceValidator;
import org.hl7.fhir.instance.model.api.IAnyResource;
@ -246,6 +248,8 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields());
myDaoConfig.setAdvancedHSearchIndexing(new DaoConfig().isAdvancedHSearchIndexing());
myModelConfig.setIndexOnContainedResources(new ModelConfig().isIndexOnContainedResources());
mySearchCoordinatorSvcRaw.setLoadingThrottleForUnitTests(null);
mySearchCoordinatorSvcRaw.setSyncSizeForUnitTests(QueryParameterUtils.DEFAULT_SYNC_SIZE);
mySearchCoordinatorSvcRaw.setNeverUseLocalSearchForUnitTests(false);
@ -3136,6 +3140,84 @@ public class ResourceProviderR4Test extends BaseResourceProviderR4Test {
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testHasParameterOnChain(boolean theWithIndexOnContainedResources) throws Exception {
myModelConfig.setIndexOnContainedResources(theWithIndexOnContainedResources);
IIdType pid0;
IIdType pid1;
IIdType groupId;
IIdType obsId;
{
Patient patient = new Patient();
patient.addIdentifier().setSystem("urn:system").setValue("001");
patient.addName().setFamily("Tester").addGiven("Joe");
pid0 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless();
}
{
Patient patient = new Patient();
patient.addIdentifier().setSystem("urn:system").setValue("001");
patient.addName().setFamily("Tester").addGiven("Joe");
pid1 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless();
}
{
Group group = new Group();
group.addMember().setEntity(new Reference(pid0.getValue()));
group.addMember().setEntity(new Reference(pid1.getValue()));
groupId = myGroupDao.create(group, mySrd).getId().toUnqualifiedVersionless();
}
{
Observation obs = new Observation();
obs.getCode().addCoding().setSystem("urn:system").setCode("FOO");
obs.getSubject().setReferenceElement(pid0);
obsId = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
}
String uri;
List<String> ids;
logAllTokenIndexes();
uri = ourServerBase + "/Observation?code=urn:system%7CFOO&subject._has:Group:member:_id=" + groupId.getValue();
myCaptureQueriesListener.clear();
ids = searchAndReturnUnqualifiedVersionlessIdValues(uri);
myCaptureQueriesListener.logAllQueries();
assertThat(ids, contains(obsId.getValue()));
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testHasParameterWithIdTarget(boolean theWithIndexOnContainedResources) throws Exception {
myModelConfig.setIndexOnContainedResources(theWithIndexOnContainedResources);
IIdType pid0;
IIdType obsId;
{
Patient patient = new Patient();
patient.addIdentifier().setSystem("urn:system").setValue("001");
patient.addName().setFamily("Tester").addGiven("Joe");
pid0 = myPatientDao.create(patient, mySrd).getId().toUnqualifiedVersionless();
}
{
Observation obs = new Observation();
obs.addIdentifier().setSystem("urn:system").setValue("FOO");
obs.getSubject().setReferenceElement(pid0);
obsId = myObservationDao.create(obs, mySrd).getId().toUnqualifiedVersionless();
}
String uri;
List<String> ids;
logAllResourceLinks();
uri = ourServerBase + "/Patient?_has:Observation:subject:_id=" + obsId.getValue();
myCaptureQueriesListener.clear();
ids = searchAndReturnUnqualifiedVersionlessIdValues(uri);
myCaptureQueriesListener.logAllQueries();
assertThat(ids, contains(pid0.getValue()));
}
@Test
public void testHistoryWithAtParameter() throws Exception {
String methodName = "testHistoryWithFromAndTo";

10
hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/RestServerR4Helper.java
View File

@ -33,6 +33,7 @@ import ca.uhn.fhir.rest.server.RestfulServer;
import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
import ca.uhn.fhir.rest.server.provider.HashMapResourceProvider;
import org.eclipse.jetty.server.Request;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle;
@ -89,9 +90,12 @@ public class RestServerR4Helper extends BaseRestServerHelper implements BeforeEa
}
public List<Bundle> getTransactions() {
return myRestServer
.getPlainProvider()
.getTransactions()
List<IBaseBundle> transactions = myRestServer.getPlainProvider().getTransactions();
// Make a copy to avoid synchronization issues
transactions = new ArrayList<>(transactions);
return transactions
.stream()
.map(t -> (Bundle) t)
.collect(Collectors.toList());