Fix fhirpath expression evaluation with _fhirpath parameter when resolve() references resources within a Bundle (#5565)

* Fix fhirpath expression evaluation with _fhirpath parameter when resolve() references resources within a Bundle

* Update parameters for getBundleReference method

* Addressing some code review comments

* Spotless fix and update test logic to address code review comment
This commit is contained in:
Martha Mitran 2024-01-09 10:15:01 -08:00 committed by GitHub
parent adfdbab0f0
commit effbc98b30
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 165 additions and 74 deletions

View File

@ -25,6 +25,7 @@ import ca.uhn.fhir.context.BaseRuntimeElementDefinition;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.RuntimeResourceDefinition;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.model.primitive.IdDt;
import ca.uhn.fhir.rest.api.PatchTypeEnum;
import ca.uhn.fhir.rest.api.RequestTypeEnum;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
@ -36,6 +37,7 @@ import ca.uhn.fhir.util.bundle.ModifiableBundleEntry;
import ca.uhn.fhir.util.bundle.SearchBundleEntryParts;
import com.google.common.collect.Sets;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import org.apache.commons.lang3.tuple.Pair;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseBinary;
@ -56,6 +58,7 @@ import java.util.Set;
import java.util.function.Consumer;
import java.util.stream.Collectors;
import static org.apache.commons.lang3.StringUtils.isBlank;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
import static org.hl7.fhir.instance.model.api.IBaseBundle.LINK_PREV;
@ -642,6 +645,41 @@ public class BundleUtil {
return retVal;
}
public static IBase getReferenceInBundle(
@Nonnull FhirContext theFhirContext, @Nonnull String theUrl, @Nullable Object theAppContext) {
if (!(theAppContext instanceof IBaseBundle) || isBlank(theUrl) || theUrl.startsWith("#")) {
return null;
}
/*
* If this is a reference that is a UUID, we must be looking for local references within a Bundle
*/
IBaseBundle bundle = (IBaseBundle) theAppContext;
final boolean isPlaceholderReference = theUrl.startsWith("urn:");
final String unqualifiedVersionlessReference =
new IdDt(theUrl).toUnqualifiedVersionless().getValue();
for (BundleEntryParts next : BundleUtil.toListOfEntries(theFhirContext, bundle)) {
IBaseResource nextResource = next.getResource();
if (nextResource == null) {
continue;
}
if (isPlaceholderReference) {
if (theUrl.equals(next.getUrl())
|| theUrl.equals(nextResource.getIdElement().getValue())) {
return nextResource;
}
} else {
if (unqualifiedVersionlessReference.equals(
nextResource.getIdElement().toUnqualifiedVersionless().getValue())) {
return nextResource;
}
}
}
return null;
}
/**
* DSTU3 did not allow the PATCH verb for transaction bundles- so instead we infer that a bundle
* is a patch if the payload is a binary resource containing a patch. This method

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 5564
title: "Previously, FHIRPath expression evaluation when using the `_fhirpath` parameter would not work on chained
use of 'resolve()'. This was most notable when using `_fhirpath` with FHIR Documents (i.e. 'Bundle' of type 'document'
where 'entry[0]' is a 'Composition'). This has now been fixed."

View File

@ -41,12 +41,10 @@ import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.RestSearchParameterTypeEnum;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
import ca.uhn.fhir.util.BundleUtil;
import ca.uhn.fhir.util.FhirTerser;
import ca.uhn.fhir.util.HapiExtensions;
import ca.uhn.fhir.util.StringUtil;
import ca.uhn.fhir.util.UrlUtil;
import ca.uhn.fhir.util.bundle.BundleEntryParts;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Sets;
import jakarta.annotation.Nonnull;
@ -59,14 +57,12 @@ import org.apache.commons.text.StringTokenizer;
import org.fhir.ucum.Pair;
import org.hl7.fhir.exceptions.FHIRException;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseBundle;
import org.hl7.fhir.instance.model.api.IBaseEnumeration;
import org.hl7.fhir.instance.model.api.IBaseExtension;
import org.hl7.fhir.instance.model.api.IBaseReference;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.instance.model.api.IPrimitiveType;
import org.hl7.fhir.r4.model.IdType;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
@ -2004,47 +2000,6 @@ public abstract class BaseSearchParamExtractor implements ISearchParamExtractor
}
}
@SuppressWarnings("unchecked")
protected final <T extends IBase> T resolveResourceInBundleWithPlaceholderId(Object theAppContext, String theUrl) {
/*
* If this is a reference that is a UUID, we must be looking for local
* references within a Bundle
*/
if (theAppContext instanceof IBaseBundle && isNotBlank(theUrl) && !theUrl.startsWith("#")) {
String unqualifiedVersionlessReference;
boolean isPlaceholderReference;
if (theUrl.startsWith("urn:")) {
isPlaceholderReference = true;
unqualifiedVersionlessReference = null;
} else {
isPlaceholderReference = false;
unqualifiedVersionlessReference =
new IdType(theUrl).toUnqualifiedVersionless().getValue();
}
List<BundleEntryParts> entries = BundleUtil.toListOfEntries(getContext(), (IBaseBundle) theAppContext);
for (BundleEntryParts next : entries) {
if (next.getResource() != null) {
if (isPlaceholderReference) {
if (theUrl.equals(next.getUrl())
|| theUrl.equals(
next.getResource().getIdElement().getValue())) {
return (T) next.getResource();
}
} else {
if (unqualifiedVersionlessReference.equals(next.getResource()
.getIdElement()
.toUnqualifiedVersionless()
.getValue())) {
return (T) next.getResource();
}
}
}
}
}
return null;
}
@FunctionalInterface
public interface IValueExtractor {

View File

@ -25,6 +25,7 @@ import ca.uhn.fhir.jpa.model.entity.StorageSettings;
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
import ca.uhn.fhir.sl.cache.Cache;
import ca.uhn.fhir.sl.cache.CacheFactory;
import ca.uhn.fhir.util.BundleUtil;
import com.google.common.annotations.VisibleForTesting;
import jakarta.annotation.PostConstruct;
import org.hl7.fhir.exceptions.FHIRException;
@ -139,7 +140,7 @@ public class SearchParamExtractorR4 extends BaseSearchParamExtractor implements
@Override
public Base resolveReference(Object theAppContext, String theUrl, Base theRefContext) throws FHIRException {
Base retVal = resolveResourceInBundleWithPlaceholderId(theAppContext, theUrl);
Base retVal = (Base) BundleUtil.getReferenceInBundle(getContext(), theUrl, theAppContext);
if (retVal != null) {
return retVal;
}

View File

@ -25,6 +25,7 @@ import ca.uhn.fhir.jpa.model.entity.StorageSettings;
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
import ca.uhn.fhir.sl.cache.Cache;
import ca.uhn.fhir.sl.cache.CacheFactory;
import ca.uhn.fhir.util.BundleUtil;
import com.google.common.annotations.VisibleForTesting;
import jakarta.annotation.PostConstruct;
import org.hl7.fhir.exceptions.FHIRException;
@ -139,7 +140,7 @@ public class SearchParamExtractorR4B extends BaseSearchParamExtractor implements
@Override
public Base resolveReference(Object theAppContext, String theUrl, Base refContext) throws FHIRException {
Base retVal = resolveResourceInBundleWithPlaceholderId(theAppContext, theUrl);
Base retVal = (Base) BundleUtil.getReferenceInBundle(getContext(), theUrl, theAppContext);
if (retVal != null) {
return retVal;
}

View File

@ -25,6 +25,7 @@ import ca.uhn.fhir.jpa.model.entity.StorageSettings;
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
import ca.uhn.fhir.sl.cache.Cache;
import ca.uhn.fhir.sl.cache.CacheFactory;
import ca.uhn.fhir.util.BundleUtil;
import jakarta.annotation.PostConstruct;
import org.hl7.fhir.exceptions.FHIRException;
import org.hl7.fhir.exceptions.PathEngineException;
@ -136,7 +137,7 @@ public class SearchParamExtractorR5 extends BaseSearchParamExtractor implements
@Override
public Base resolveReference(Object appContext, String theUrl, Base refContext) throws FHIRException {
Base retVal = resolveResourceInBundleWithPlaceholderId(appContext, theUrl);
Base retVal = (Base) BundleUtil.getReferenceInBundle(getContext(), theUrl, appContext);
if (retVal != null) {
return retVal;
}

View File

@ -22,6 +22,7 @@ package ca.uhn.fhir.rest.server.interceptor;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.fhirpath.FhirPathExecutionException;
import ca.uhn.fhir.fhirpath.IFhirPath;
import ca.uhn.fhir.fhirpath.IFhirPathEvaluationContext;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.interceptor.api.Hook;
import ca.uhn.fhir.interceptor.api.Pointcut;
@ -29,10 +30,14 @@ import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.ResponseDetails;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.util.BundleUtil;
import ca.uhn.fhir.util.ParametersUtil;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseParameters;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import java.util.List;
@ -65,6 +70,12 @@ public class FhirPathFilterInterceptor {
ParametersUtil.addPartString(ctx, resultPart, "expression", expression);
IFhirPath fhirPath = ctx.newFhirPath();
fhirPath.setEvaluationContext(new IFhirPathEvaluationContext() {
@Override
public IBase resolveReference(@Nonnull IIdType theReference, @Nullable IBase theContext) {
return BundleUtil.getReferenceInBundle(ctx, theReference.getValue(), responseResource);
}
});
List<IBase> outputs;
try {
outputs = fhirPath.evaluate(responseResource, expression, IBase.class);

View File

@ -1,7 +1,6 @@
package ca.uhn.fhir.rest.server.interceptor;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.test.utilities.HttpClientExtension;
@ -13,40 +12,63 @@ import org.apache.commons.io.IOUtils;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.CodeableConcept;
import org.hl7.fhir.r4.model.Coding;
import org.hl7.fhir.r4.model.Composition;
import org.hl7.fhir.r4.model.DateTimeType;
import org.hl7.fhir.r4.model.MedicationAdministration;
import org.hl7.fhir.r4.model.Parameters;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.Period;
import org.hl7.fhir.r4.model.Reference;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.stream.Stream;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.not;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class FhirPathFilterInterceptorTest {
public class FhirPathFilterInterceptorR4Test {
private static final Logger ourLog = LoggerFactory.getLogger(FhirPathFilterInterceptorTest.class);
private static final Logger ourLog = LoggerFactory.getLogger(FhirPathFilterInterceptorR4Test.class);
private static FhirContext ourCtx = FhirContext.forR4();
@Order(0)
@RegisterExtension
public HttpClientExtension myHttpClientExtension = new HttpClientExtension();
@Order(0)
@RegisterExtension
public RestfulServerExtension myServerExtension = new RestfulServerExtension(ourCtx);
@Order(1)
@RegisterExtension
public HashMapResourceProviderExtension<Patient> myProviderExtension = new HashMapResourceProviderExtension<>(myServerExtension, Patient.class);
public HashMapResourceProviderExtension<Patient> myPatientProvider = new HashMapResourceProviderExtension<>(myServerExtension, Patient.class);
@Order(1)
@RegisterExtension
public HashMapResourceProviderExtension<MedicationAdministration> myMedicationAdministrationProvider = new HashMapResourceProviderExtension<>(myServerExtension, MedicationAdministration.class);
@Order(1)
@RegisterExtension
public HashMapResourceProviderExtension<Bundle> myBundleProvider = new HashMapResourceProviderExtension<>(myServerExtension, Bundle.class);
private IGenericClient myClient;
private String myBaseUrl;
private CloseableHttpClient myHttpClient;
private IIdType myPatientId;
@BeforeEach
public void before() {
myProviderExtension.clear();
myServerExtension.getRestfulServer().getInterceptorService().unregisterAllInterceptors();
myServerExtension.getRestfulServer().getInterceptorService().registerInterceptor(new FhirPathFilterInterceptor());
@ -57,9 +79,9 @@ public class FhirPathFilterInterceptorTest {
@Test
public void testUnfilteredResponse() throws IOException {
createPatient();
IIdType patientId = createPatient();
HttpGet request = new HttpGet(myPatientId.getValue());
HttpGet request = new HttpGet(patientId.getValue());
try (CloseableHttpResponse response = myHttpClient.execute(request)) {
String responseText = IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response:\n{}", responseText);
@ -72,9 +94,9 @@ public class FhirPathFilterInterceptorTest {
@Test
public void testUnfilteredResponse_WithResponseHighlightingInterceptor() throws IOException {
myServerExtension.getRestfulServer().registerInterceptor(new ResponseHighlighterInterceptor());
createPatient();
final IIdType patientId = createPatient();
HttpGet request = new HttpGet(myPatientId.getValue() + "?_format=" + Constants.FORMATS_HTML_JSON);
HttpGet request = new HttpGet(patientId.getValue() + "?_format=" + Constants.FORMATS_HTML_JSON);
try (CloseableHttpResponse response = myHttpClient.execute(request)) {
String responseText = IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response:\n{}", responseText);
@ -85,9 +107,9 @@ public class FhirPathFilterInterceptorTest {
@Test
public void testFilteredResponse() throws IOException {
createPatient();
final IIdType patientId = createPatient();
HttpGet request = new HttpGet(myPatientId + "?_fhirpath=Patient.identifier&_pretty=true");
HttpGet request = new HttpGet(patientId + "?_fhirpath=Patient.identifier&_pretty=true");
try (CloseableHttpResponse response = myHttpClient.execute(request)) {
String responseText = IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response:\n{}", responseText);
@ -99,9 +121,9 @@ public class FhirPathFilterInterceptorTest {
@Test
public void testFilteredResponse_ExpressionReturnsExtension() throws IOException {
createPatient();
final IIdType patientId = createPatient();
HttpGet request = new HttpGet(myPatientId + "?_fhirpath=Patient.extension('http://hl7.org/fhir/us/core/StructureDefinition/us-core-race')&_pretty=true");
HttpGet request = new HttpGet(patientId + "?_fhirpath=Patient.extension('http://hl7.org/fhir/us/core/StructureDefinition/us-core-race')&_pretty=true");
try (CloseableHttpResponse response = myHttpClient.execute(request)) {
String responseText = IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response:\n{}", responseText);
@ -112,9 +134,9 @@ public class FhirPathFilterInterceptorTest {
@Test
public void testFilteredResponse_ExpressionReturnsResource() throws IOException {
createPatient();
final IIdType patientId = createPatient();
HttpGet request = new HttpGet(myPatientId + "?_fhirpath=Patient&_pretty=true");
HttpGet request = new HttpGet(patientId + "?_fhirpath=Patient&_pretty=true");
try (CloseableHttpResponse response = myHttpClient.execute(request)) {
String responseText = IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response:\n{}", responseText);
@ -127,9 +149,9 @@ public class FhirPathFilterInterceptorTest {
@Test
public void testFilteredResponse_ExpressionIsInvalid() throws IOException {
createPatient();
final IIdType patientId = createPatient();
HttpGet request = new HttpGet(myPatientId + "?_fhirpath=" + UrlUtil.escapeUrlParam("***"));
HttpGet request = new HttpGet(patientId + "?_fhirpath=" + UrlUtil.escapeUrlParam("***"));
try (CloseableHttpResponse response = myHttpClient.execute(request)) {
String responseText = IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response:\n{}", responseText);
@ -160,9 +182,9 @@ public class FhirPathFilterInterceptorTest {
@Test
public void testFilteredResponse_WithResponseHighlightingInterceptor() throws IOException {
myServerExtension.getRestfulServer().registerInterceptor(new ResponseHighlighterInterceptor());
createPatient();
final IIdType patientId = createPatient();
HttpGet request = new HttpGet(myPatientId + "?_fhirpath=Patient.identifier&_format=" + Constants.FORMATS_HTML_JSON);
HttpGet request = new HttpGet(patientId + "?_fhirpath=Patient.identifier&_format=" + Constants.FORMATS_HTML_JSON);
try (CloseableHttpResponse response = myHttpClient.execute(request)) {
String responseText = IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response:\n{}", responseText);
@ -172,19 +194,75 @@ public class FhirPathFilterInterceptorTest {
}
private void createPatient() {
public static Stream<Arguments> getBundleParameters() {
return Stream.of(
Arguments.of("Bundle.entry.resource.type", "valueCodeableConcept"),
Arguments.of("Bundle.entry.resource.ofType(Patient).identifier", "valueIdentifier"),
Arguments.of("Bundle.entry.resource.ofType(MedicationAdministration).effective", "valuePeriod"),
Arguments.of("Bundle.entry[0].resource.as(Composition).type", "valueCodeableConcept"),
Arguments.of("Bundle.entry[0].resource.as(Composition).subject.resolve().as(Patient).identifier", "valueIdentifier"),
Arguments.of("Bundle.entry[0].resource.as(Composition).section.entry.resolve().as(MedicationAdministration).effective", "valuePeriod")
);
}
@ParameterizedTest
@MethodSource(value = "getBundleParameters")
public void testFilteredResponse_withBundleComposition_returnsResult(final String theFhirPathExpression, final String expectedResult) throws IOException {
IIdType bundle = createBundleDocument();
HttpGet request = new HttpGet(bundle.getValue() + "?_fhirpath=" + theFhirPathExpression);
try (CloseableHttpResponse response = myHttpClient.execute(request)) {
String responseText = IOUtils.toString(response.getEntity().getContent(), Charsets.UTF_8);
ourLog.info("Response:\n{}", responseText);
IBaseResource resource = ourCtx.newJsonParser().parseResource(responseText);
assertTrue(resource instanceof Parameters);
Parameters parameters = (Parameters)resource;
Parameters.ParametersParameterComponent parameterComponent = parameters.getParameter("result");
assertNotNull(parameterComponent);
assertEquals(2, parameterComponent.getPart().size());
Parameters.ParametersParameterComponent resultComponent = parameterComponent.getPart().get(1);
assertEquals("result", resultComponent.getName());
assertThat(responseText, containsString(expectedResult));
}
}
private IIdType createPatient() {
Patient p = new Patient();
p.addExtension()
.setUrl("http://hl7.org/fhir/us/core/StructureDefinition/us-core-race")
.addExtension()
.setUrl("ombCategory")
.setValue(new Coding("urn:oid:2.16.840.1.113883.6.238", "2106-3", "White"));
.setUrl("http://hl7.org/fhir/us/core/StructureDefinition/us-core-race")
.addExtension()
.setUrl("ombCategory")
.setValue(new Coding("urn:oid:2.16.840.1.113883.6.238", "2106-3", "White"));
p.setActive(true);
p.addIdentifier().setSystem("http://identifiers/1").setValue("value-1");
p.addIdentifier().setSystem("http://identifiers/2").setValue("value-2");
p.addName().setFamily("Simpson").addGiven("Homer").addGiven("Jay");
p.addName().setFamily("Simpson").addGiven("Grandpa");
myPatientId = myClient.create().resource(p).execute().getId().withServerBase(myBaseUrl, "Patient");
return myClient.create().resource(p).execute().getId().withServerBase(myBaseUrl, "Patient");
}
private IIdType createBundleDocument() {
Patient patient = new Patient();
patient.setActive(true);
patient.addIdentifier().setSystem("http://identifiers/1").setValue("value-1");
patient.addName().setFamily("Simpson").addGiven("Homer").addGiven("Jay");
patient = (Patient) myClient.create().resource(patient).execute().getResource();
MedicationAdministration medicationAdministration = new MedicationAdministration();
medicationAdministration.setEffective(new Period().setStartElement(DateTimeType.now()));
medicationAdministration = (MedicationAdministration) myClient.create().resource(medicationAdministration).execute().getResource();
Bundle bundle = new Bundle();
bundle.setType(Bundle.BundleType.DOCUMENT);
Composition composition = new Composition();
composition.setType(new CodeableConcept().addCoding(new Coding().setCode("code").setSystem("http://example.org")));
bundle.addEntry().setResource(composition);
composition.getSubject().setReference(patient.getIdElement().getValue());
composition.addSection().addEntry(new Reference(medicationAdministration.getIdElement()));
bundle.addEntry().setResource(patient);
bundle.addEntry().setResource(medicationAdministration);
return myClient.create().resource(bundle).execute().getId().withServerBase(myBaseUrl, "Bundle");
}
}