Fix '_include' to work for canonical urls (#3440)

* Added test to verify BUG: #2843 (_include doesn't return canonically referred resource)

* Fix '_include' query search by native query to support canonical url references

* Fixed code review comments
This commit is contained in:
Jaison Baskaran 2022-03-03 21:28:19 -07:00 committed by GitHub
parent 20e092ba6d
commit ecf782d59e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 176 additions and 84 deletions

View File

@ -20,12 +20,12 @@ package ca.uhn.fhir.jpa.search.builder;
* #L%
*/
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.context.ComboSearchParamType;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.FhirVersionEnum;
import ca.uhn.fhir.context.RuntimeResourceDefinition;
import ca.uhn.fhir.context.RuntimeSearchParam;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.interceptor.api.HookParams;
import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
import ca.uhn.fhir.interceptor.api.Pointcut;
@ -96,6 +96,7 @@ import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
import com.healthmarketscience.sqlbuilder.Condition;
import org.apache.commons.lang3.Validate;
import org.apache.commons.lang3.math.NumberUtils;
import org.hl7.fhir.instance.model.api.IAnyResource;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.slf4j.Logger;
@ -109,6 +110,9 @@ import javax.annotation.Nonnull;
import javax.persistence.EntityManager;
import javax.persistence.PersistenceContext;
import javax.persistence.PersistenceContextType;
import javax.persistence.Query;
import javax.persistence.SqlResultSetMapping;
import javax.persistence.Tuple;
import javax.persistence.TypedQuery;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
@ -148,6 +152,11 @@ public class SearchBuilder implements ISearchBuilder {
public static final int MAXIMUM_PAGE_SIZE_FOR_TESTING = 50;
private static final Logger ourLog = LoggerFactory.getLogger(SearchBuilder.class);
private static final ResourcePersistentId NO_MORE = new ResourcePersistentId(-1L);
private static final String MY_TARGET_RESOURCE_PID = "myTargetResourcePid";
private static final String MY_SOURCE_RESOURCE_PID = "mySourceResourcePid";
private static final String MY_TARGET_RESOURCE_VERSION = "myTargetResourceVersion";
public static final String RESOURCE_ID_ALIAS = "resource_id";
public static final String RESOURCE_VERSION_ALIAS = "resource_version";
public static boolean myUseMaxPageSize50ForTest = false;
private final String myResourceName;
private final Class<? extends IBaseResource> myResourceType;
@ -888,11 +897,11 @@ public class SearchBuilder implements ISearchBuilder {
if (theIncludes == null || theIncludes.isEmpty()) {
return new HashSet<>();
}
String searchPidFieldName = theReverseMode ? "myTargetResourcePid" : "mySourceResourcePid";
String findPidFieldName = theReverseMode ? "mySourceResourcePid" : "myTargetResourcePid";
String searchPidFieldName = theReverseMode ? MY_TARGET_RESOURCE_PID : MY_SOURCE_RESOURCE_PID;
String findPidFieldName = theReverseMode ? MY_SOURCE_RESOURCE_PID : MY_TARGET_RESOURCE_PID;
String findVersionFieldName = null;
if (!theReverseMode && myModelConfig.isRespectVersionsForSearchIncludes()) {
findVersionFieldName = "myTargetResourceVersion";
findVersionFieldName = MY_TARGET_RESOURCE_VERSION;
}
List<ResourcePersistentId> nextRoundMatches = new ArrayList<>(theMatches);
@ -1011,25 +1020,55 @@ public class SearchBuilder implements ISearchBuilder {
String targetResourceType = defaultString(nextInclude.getParamTargetType(), null);
for (String nextPath : paths) {
String sql;
boolean haveTargetTypesDefinedByParam = param.hasTargets();
String fieldsToLoad = "r." + findPidFieldName;
String findPidFieldSqlColumn = findPidFieldName.equals(MY_SOURCE_RESOURCE_PID) ? "src_resource_id" : "target_resource_id";
String fieldsToLoad = "r." + findPidFieldSqlColumn + " AS " + RESOURCE_ID_ALIAS;
if (findVersionFieldName != null) {
fieldsToLoad += ", r." + findVersionFieldName;
fieldsToLoad += ", r.target_resource_version AS " + RESOURCE_VERSION_ALIAS;
}
if (targetResourceType != null) {
sql = "SELECT " + fieldsToLoad + " FROM ResourceLink r WHERE r.mySourcePath = :src_path AND r." + searchPidFieldName + " IN (:target_pids) AND r.myTargetResourceType = :target_resource_type";
} else if (haveTargetTypesDefinedByParam) {
sql = "SELECT " + fieldsToLoad + " FROM ResourceLink r WHERE r.mySourcePath = :src_path AND r." + searchPidFieldName + " IN (:target_pids) AND r.myTargetResourceType in (:target_resource_types)";
} else {
sql = "SELECT " + fieldsToLoad + " FROM ResourceLink r WHERE r.mySourcePath = :src_path AND r." + searchPidFieldName + " IN (:target_pids)";
// Query for includes lookup has consider 2 cases
// Case 1: Where target_resource_id is available in hfj_res_link table for local references
// Case 2: Where target_resource_id is null in hfj_res_link table and referred by a canonical url in target_resource_url
// Case 1:
String searchPidFieldSqlColumn = searchPidFieldName.equals(MY_TARGET_RESOURCE_PID) ? "target_resource_id" : "src_resource_id";
StringBuilder resourceIdBasedQuery = new StringBuilder("SELECT " + fieldsToLoad +
" FROM hfj_res_link r " +
" WHERE r.src_path = :src_path AND " +
" r.target_resource_id IS NOT NULL AND " +
" r." + searchPidFieldSqlColumn + " IN (:target_pids) ");
if(targetResourceType != null) {
resourceIdBasedQuery.append(" AND r.target_resource_type = :target_resource_type ");
} else if(haveTargetTypesDefinedByParam) {
resourceIdBasedQuery.append(" AND r.target_resource_type in (:target_resource_types) ");
}
// Case 2:
String fieldsToLoadFromSpidxUriTable = "rUri.res_id";
// to match the fields loaded in union
if(fieldsToLoad.split(",").length > 1) {
for (int i = 0; i < fieldsToLoad.split(",").length - 1; i++) {
fieldsToLoadFromSpidxUriTable += ", NULL";
}
}
//@formatter:off
StringBuilder resourceUrlBasedQuery = new StringBuilder("SELECT " + fieldsToLoadFromSpidxUriTable +
" FROM hfj_res_link r " +
" JOIN hfj_spidx_uri rUri ON ( " +
" r.target_resource_url = rUri.sp_uri AND " +
" rUri.sp_name = 'url' " +
" ) " +
" WHERE r.src_path = :src_path AND " +
" r.target_resource_id IS NULL AND " +
" r." + searchPidFieldSqlColumn + " IN (:target_pids) ");
//@formatter:on
String sql = resourceIdBasedQuery + " UNION " + resourceUrlBasedQuery;
List<Collection<ResourcePersistentId>> partitions = partition(nextRoundMatches, getMaximumPageSize());
for (Collection<ResourcePersistentId> nextPartition : partitions) {
TypedQuery<?> q = theEntityManager.createQuery(sql, Object[].class);
Query q = theEntityManager.createNativeQuery(sql, Tuple.class);
q.setParameter("src_path", nextPath);
q.setParameter("target_pids", ResourcePersistentId.toLongList(nextPartition));
if (targetResourceType != null) {
@ -1037,21 +1076,18 @@ public class SearchBuilder implements ISearchBuilder {
} else if (haveTargetTypesDefinedByParam) {
q.setParameter("target_resource_types", param.getTargets());
}
List<?> results = q.getResultList();
List<Tuple> results = q.getResultList();
if (theMaxCount != null) {
q.setMaxResults(theMaxCount);
}
for (Object resourceLink : results) {
if (resourceLink != null) {
ResourcePersistentId persistentId;
if (findVersionFieldName != null) {
persistentId = new ResourcePersistentId(((Object[]) resourceLink)[0]);
persistentId.setVersion((Long) ((Object[]) resourceLink)[1]);
} else {
persistentId = new ResourcePersistentId(resourceLink);
for (Tuple result : results) {
if (result != null) {
Long resourceId = NumberUtils.createLong(String.valueOf(result.get(RESOURCE_ID_ALIAS)));
Long resourceVersion = null;
if (findVersionFieldName != null && result.get(RESOURCE_VERSION_ALIAS) != null) {
resourceVersion = NumberUtils.createLong(String.valueOf(result.get(RESOURCE_VERSION_ALIAS)));
}
assert persistentId.getId() instanceof Long;
pidsToInclude.add(persistentId);
pidsToInclude.add(new ResourcePersistentId(resourceId, resourceVersion));
}
}
}

View File

@ -14,20 +14,28 @@ import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.Coding;
import org.hl7.fhir.r4.model.DateType;
import org.hl7.fhir.r4.model.DecimalType;
import org.hl7.fhir.r4.model.Enumerations;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Identifier;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.Questionnaire;
import org.hl7.fhir.r4.model.Questionnaire.QuestionnaireItemType;
import org.hl7.fhir.r4.model.QuestionnaireResponse;
import org.hl7.fhir.r4.model.QuestionnaireResponse.QuestionnaireResponseStatus;
import org.hl7.fhir.r4.model.StringType;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
@ -71,12 +79,12 @@ public class ResourceProviderQuestionnaireResponseR4Test extends BaseResourcePro
public void testCreateWithNonLocalReferenceWorksWithIncludes() {
String baseUrl = "https://hapi.fhir.org/baseR4/";
myModelConfig.setTreatBaseUrlsAsLocal(Collections.singleton(baseUrl));
Questionnaire questionnaire = new Questionnaire();
questionnaire.setUrl(baseUrl + "Questionnaire/my-questionnaire");
questionnaire.setId("my-questionnaire");
QuestionnaireResponse questionnaireResponse = new QuestionnaireResponse();
questionnaireResponse.setQuestionnaire(baseUrl + "Questionnaire/my-questionnaire");
questionnaireResponse.setQuestionnaire(questionnaire.getUrl());
questionnaireResponse.setId("my-questionnaire-response");
myQuestionnaireDao.update(questionnaire);
@ -100,7 +108,7 @@ public class ResourceProviderQuestionnaireResponseR4Test extends BaseResourcePro
Questionnaire q1 = new Questionnaire();
q1.addItem().setLinkId("link1").setType(QuestionnaireItemType.STRING);
IIdType qId = myQuestionnaireDao.create(q1, mySrd).getId().toUnqualifiedVersionless();
QuestionnaireResponse qr1 = new QuestionnaireResponse();
qr1.setQuestionnaire(qId.getValue());
qr1.setStatus(QuestionnaireResponseStatus.COMPLETED);
@ -112,49 +120,99 @@ public class ResourceProviderQuestionnaireResponseR4Test extends BaseResourcePro
assertThat(myFhirContext.newJsonParser().encodeResourceToString(e.getOperationOutcome()), containsString("Answer value must be of type string"));
}
}
/**
* Test added to verify <a href="https://github.com/hapifhir/hapi-fhir/issues/2843">https://github.com/hapifhir/hapi-fhir/issues/2843</a>
*/
@Test
public void testSearch_withIncludeQuestionnaire_shouldReturnWithCanonicalReferencedQuestionnaire() {
String questionnaireCanonicalUrl = "https://hapi.fhir.org/baseR4/Questionnaire/xl-54127-6-hapi";
Questionnaire questionnaire = new Questionnaire();
questionnaire.setId("xl-54127-6-hapi");
questionnaire.setUrl(questionnaireCanonicalUrl);
questionnaire.addIdentifier(new Identifier().setSystem("http://loinc.org").setValue("54127-6"));
questionnaire.setName("US Surgeon General family health portrait");
questionnaire.setTitle(questionnaire.getName());
questionnaire.setStatus(Enumerations.PublicationStatus.DRAFT);
questionnaire.addSubjectType("Patient").addSubjectType("Person");
questionnaire.addCode(new Coding("http://loinc.org", "54127-6", questionnaire.getName()));
questionnaire.addItem().setLinkId("/54126-8")
.setType(QuestionnaireItemType.GROUP)
.setRequired(false)
.setText("My health history")
.addCode(new Coding("http://loinc.org", "54126-8", "My health history"));
IIdType qIdType = myQuestionnaireDao.create(questionnaire, mySrd).getId().toUnqualifiedVersionless();
QuestionnaireResponse questionnaireResponse = new QuestionnaireResponse();
questionnaireResponse.setId("xl-54127-6-hapi-response");
questionnaireResponse.setQuestionnaire(questionnaireCanonicalUrl);
questionnaireResponse.setStatus(QuestionnaireResponseStatus.COMPLETED);
questionnaireResponse.addItem().setLinkId("/54126-8")
.setText("My health history")
.addItem().setLinkId("/54126-8/54125-0").setText("Name").addAnswer().setValue(new StringType("TAMBRA AGARWAL"))
.addItem().setLinkId("/54126-8/21112-8").setText("Birth Date").addAnswer().setValue(new DateType("1994-01-01"));
IIdType qrIdType = myQuestionnaireResponseDao.create(questionnaireResponse, mySrd).getId().toUnqualifiedVersionless();
// Search
Bundle results = myClient.search()
.byUrl("QuestionnaireResponse?_id=" + qrIdType.toUnqualifiedVersionless() + "&_include=QuestionnaireResponse:questionnaire")
.returnBundle(Bundle.class)
.execute();
assertThat(results.getEntry().size(), is(equalTo(2)));
List<String> expectedIds = new ArrayList<>();
expectedIds.add(qrIdType.getValue());
expectedIds.add(qIdType.getValue());
List<String> actualIds = results.getEntry().stream()
.map(Bundle.BundleEntryComponent::getResource)
.map(resource -> resource.getIdElement().toUnqualifiedVersionless().toString())
.collect(Collectors.toList());
assertEquals(expectedIds, actualIds);
}
@Test
public void testSaveQuestionnaire() throws Exception {
String input = "<QuestionnaireResponse xmlns=\"http://hl7.org/fhir\">\n" +
" <status value=\"completed\"/>\n" +
" <authored value=\"2016-05-03T13:05:20-04:00\"/>\n" +
" <item>\n" +
" <linkId value=\"breast-feeding-intention\"/>\n" +
" <text value=\"Breast Feeding Intention:\"/>\n" +
" <answer>\n" +
" <valueCoding>\n" +
" <system value=\"http://example.org/codesystem-breastfeeding-intention\"/>\n" +
" <code value=\"true\"/>\n" +
" <display value=\"Mother wants to provide formula exclusively\"/>\n" +
" </valueCoding>\n" +
" </answer>\n" +
" </item>\n" +
" <item>\n" +
" <linkId value=\"breast-feeding-education\"/>\n" +
" <text value=\"Answer if not exclusive BM:\"/>\n" +
" <answer>\n" +
" <valueCoding>\n" +
" <system value=\"http://example.org/codesystem-breastfeeding-education\"/>\n" +
" <code value=\"true\"/>\n" +
" <display value=\"Mother not given comprehensive education per protocol\"/>\n" +
" </valueCoding>\n" +
" </answer>\n" +
" </item>\n" +
" <item>\n" +
" <linkId value=\"breast-feeding-exclusion\"/>\n" +
" <text value=\"Exclusion Criteria:\"/>\n" +
" <answer>\n" +
" <valueCoding>\n" +
" <system value=\"http://example.org/codesystem-breastfeeding-exclusion\"/>\n" +
" <code value=\"true\"/>\n" +
" <display\n" +
" value=\"Maternal use of drugs of abuse, antimetabolites, chemotherapeutic agents, or radioisotopes\"\n" +
" />\n" +
" </valueCoding>\n" +
" </answer>\n" +
" </item>\n" +
"</QuestionnaireResponse>";
String input = "<QuestionnaireResponse xmlns=\"http://hl7.org/fhir\">\n" +
" <status value=\"completed\"/>\n" +
" <authored value=\"2016-05-03T13:05:20-04:00\"/>\n" +
" <item>\n" +
" <linkId value=\"breast-feeding-intention\"/>\n" +
" <text value=\"Breast Feeding Intention:\"/>\n" +
" <answer>\n" +
" <valueCoding>\n" +
" <system value=\"http://example.org/codesystem-breastfeeding-intention\"/>\n" +
" <code value=\"true\"/>\n" +
" <display value=\"Mother wants to provide formula exclusively\"/>\n" +
" </valueCoding>\n" +
" </answer>\n" +
" </item>\n" +
" <item>\n" +
" <linkId value=\"breast-feeding-education\"/>\n" +
" <text value=\"Answer if not exclusive BM:\"/>\n" +
" <answer>\n" +
" <valueCoding>\n" +
" <system value=\"http://example.org/codesystem-breastfeeding-education\"/>\n" +
" <code value=\"true\"/>\n" +
" <display value=\"Mother not given comprehensive education per protocol\"/>\n" +
" </valueCoding>\n" +
" </answer>\n" +
" </item>\n" +
" <item>\n" +
" <linkId value=\"breast-feeding-exclusion\"/>\n" +
" <text value=\"Exclusion Criteria:\"/>\n" +
" <answer>\n" +
" <valueCoding>\n" +
" <system value=\"http://example.org/codesystem-breastfeeding-exclusion\"/>\n" +
" <code value=\"true\"/>\n" +
" <display\n" +
" value=\"Maternal use of drugs of abuse, antimetabolites, chemotherapeutic agents, or radioisotopes\"\n" +
" />\n" +
" </valueCoding>\n" +
" </answer>\n" +
" </item>\n" +
"</QuestionnaireResponse>";
HttpPost post = new HttpPost(ourServerBase + "/QuestionnaireResponse");
post.setEntity(new StringEntity(input, ContentType.create(ca.uhn.fhir.rest.api.Constants.CT_FHIR_XML, "UTF-8")));
CloseableHttpResponse response = ourHttpClient.execute(post);
@ -179,9 +237,8 @@ public class ResourceProviderQuestionnaireResponseR4Test extends BaseResourcePro
} finally {
IOUtils.closeQuietly(response);
}
}
@Test
@ -196,16 +253,16 @@ public class ResourceProviderQuestionnaireResponseR4Test extends BaseResourcePro
} finally {
IOUtils.closeQuietly(response);
}
}
/**
* From a Skype message from Brian Postlethwaite
*/
@Test
public void testValidateQuestionnaireResponseWithNoIdForCreate() throws Exception {
String input = "{\"resourceType\":\"Parameters\",\"parameter\":[{\"name\":\"mode\",\"valueString\":\"create\"},{\"name\":\"resource\",\"resource\":{\"resourceType\":\"QuestionnaireResponse\",\"questionnaire\":\"http://fhirtest.uhn.ca/baseDstu2/Questionnaire/MedsCheckEligibility\",\"text\":{\"status\":\"generated\",\"div\":\"<div xmlns=\\\"http://www.w3.org/1999/xhtml\\\">!-- populated from the rendered HTML below --></div>\"},\"status\":\"completed\",\"authored\":\"2017-02-10T00:02:58.098Z\"}}]}";
HttpPost post = new HttpPost(ourServerBase + "/QuestionnaireResponse/$validate?_pretty=true");
post.setEntity(new StringEntity(input, ContentType.APPLICATION_JSON));
@ -217,15 +274,15 @@ public class ResourceProviderQuestionnaireResponseR4Test extends BaseResourcePro
} finally {
IOUtils.closeQuietly(response);
}
}
/**
* From a Skype message from Brian Postlethwaite
*/
@Test
public void testValidateQuestionnaireResponseWithNoIdForUpdate() throws Exception {
String input = "{\"resourceType\":\"Parameters\",\"parameter\":[{\"name\":\"mode\",\"valueString\":\"update\"},{\"name\":\"resource\",\"resource\":{\"resourceType\":\"QuestionnaireResponse\",\"questionnaire\":\"http://fhirtest.uhn.ca/baseDstu2/Questionnaire/MedsCheckEligibility\",\"text\":{\"status\":\"generated\",\"div\":\"<div xmlns=\\\"http://www.w3.org/1999/xhtml\\\">!-- populated from the rendered HTML below --></div>\"},\"status\":\"completed\",\"authored\":\"2017-02-10T00:02:58.098Z\"}}]}";
HttpPost post = new HttpPost(ourServerBase + "/QuestionnaireResponse/$validate?_pretty=true");
post.setEntity(new StringEntity(input, ContentType.APPLICATION_JSON));
@ -238,9 +295,8 @@ public class ResourceProviderQuestionnaireResponseR4Test extends BaseResourcePro
} finally {
IOUtils.closeQuietly(response);
}
}
}