Fix #1250 - Don't include Bundle.entry.response on search results

This commit is contained in:
jamesagnew 2019-06-22 16:23:32 -04:00
parent 58e6b7f6aa
commit 638cb976fd
5 changed files with 76 additions and 34 deletions

View File

@ -36,6 +36,8 @@ public enum BundleTypeEnum {
MESSAGE("message", "http://hl7.org/fhir/bundle-type"), MESSAGE("message", "http://hl7.org/fhir/bundle-type"),
BATCH_RESPONSE("batch-response", "http://hl7.org/fhir/bundle-type"),
TRANSACTION_RESPONSE("transaction-response", "http://hl7.org/fhir/bundle-type"), TRANSACTION_RESPONSE("transaction-response", "http://hl7.org/fhir/bundle-type"),
HISTORY("history", "http://hl7.org/fhir/bundle-type"), HISTORY("history", "http://hl7.org/fhir/bundle-type"),

View File

@ -27,6 +27,7 @@ import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum;
import ca.uhn.fhir.model.valueset.BundleTypeEnum; import ca.uhn.fhir.model.valueset.BundleTypeEnum;
import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.Constants;
import ca.uhn.fhir.rest.api.IVersionSpecificBundleFactory; import ca.uhn.fhir.rest.api.IVersionSpecificBundleFactory;
import ca.uhn.fhir.rest.server.RestfulServerUtils;
import ca.uhn.fhir.util.ResourceReferenceInfo; import ca.uhn.fhir.util.ResourceReferenceInfo;
import org.hl7.fhir.dstu3.model.Bundle; import org.hl7.fhir.dstu3.model.Bundle;
import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent; import org.hl7.fhir.dstu3.model.Bundle.BundleEntryComponent;
@ -127,9 +128,9 @@ public class Dstu3BundleFactory implements IVersionSpecificBundleFactory {
} }
/* /*
* Actually add the resources to the bundle * Actually add the resources to the bundle
*/ */
for (IBaseResource next : includedResources) { for (IBaseResource next : includedResources) {
BundleEntryComponent entry = myBundle.addEntry(); BundleEntryComponent entry = myBundle.addEntry();
entry.setResource((Resource) next).getSearch().setMode(SearchEntryMode.INCLUDE); entry.setResource((Resource) next).getSearch().setMode(SearchEntryMode.INCLUDE);
@ -199,7 +200,7 @@ public class Dstu3BundleFactory implements IVersionSpecificBundleFactory {
includedResources.addAll(addedResourcesThisPass); includedResources.addAll(addedResourcesThisPass);
// Linked resources may themselves have linked resources // Linked resources may themselves have linked resources
references = new ArrayList<ResourceReferenceInfo>(); references = new ArrayList<>();
for (IAnyResource iResource : addedResourcesThisPass) { for (IAnyResource iResource : addedResourcesThisPass) {
List<ResourceReferenceInfo> newReferences = myContext.newTerser().getAllResourceReferences(iResource); List<ResourceReferenceInfo> newReferences = myContext.newTerser().getAllResourceReferences(iResource);
references.addAll(newReferences); references.addAll(newReferences);
@ -220,15 +221,33 @@ public class Dstu3BundleFactory implements IVersionSpecificBundleFactory {
entry.setResource(null); entry.setResource(null);
} }
// Populate Bundle.entry.response
switch (theBundleType) {
case BATCH_RESPONSE:
case TRANSACTION_RESPONSE:
case HISTORY:
if ("1".equals(id.getVersionIdPart())) {
entry.getResponse().setStatus("201 Created");
} else if (isNotBlank(id.getVersionIdPart())) {
entry.getResponse().setStatus("200 OK");
}
if (isNotBlank(id.getVersionIdPart())) {
entry.getResponse().setEtag(RestfulServerUtils.createEtag(id.getVersionIdPart()));
}
break;
}
// Populate Bundle.entry.search
String searchMode = ResourceMetadataKeyEnum.ENTRY_SEARCH_MODE.get(nextAsResource); String searchMode = ResourceMetadataKeyEnum.ENTRY_SEARCH_MODE.get(nextAsResource);
if (searchMode != null) { if (searchMode != null) {
entry.getSearch().getModeElement().setValueAsString(searchMode); entry.getSearch().getModeElement().setValueAsString(searchMode);
} }
} }
/* /*
* Actually add the resources to the bundle * Actually add the resources to the bundle
*/ */
for (IAnyResource next : includedResources) { for (IAnyResource next : includedResources) {
BundleEntryComponent entry = myBundle.addEntry(); BundleEntryComponent entry = myBundle.addEntry();
entry.setResource((Resource) next).getSearch().setMode(SearchEntryMode.INCLUDE); entry.setResource((Resource) next).getSearch().setMode(SearchEntryMode.INCLUDE);

View File

@ -223,16 +223,23 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory {
entry.setResource(null); entry.setResource(null);
} }
// Populate Response // Populate Bundle.entry.response
if ("1".equals(id.getVersionIdPart())) { switch (theBundleType) {
entry.getResponse().setStatus("201 Created"); case BATCH_RESPONSE:
} else if (isNotBlank(id.getVersionIdPart())) { case TRANSACTION_RESPONSE:
entry.getResponse().setStatus("200 OK"); case HISTORY:
} if ("1".equals(id.getVersionIdPart())) {
if (isNotBlank(id.getVersionIdPart())) { entry.getResponse().setStatus("201 Created");
entry.getResponse().setEtag(RestfulServerUtils.createEtag(id.getVersionIdPart())); } else if (isNotBlank(id.getVersionIdPart())) {
entry.getResponse().setStatus("200 OK");
}
if (isNotBlank(id.getVersionIdPart())) {
entry.getResponse().setEtag(RestfulServerUtils.createEtag(id.getVersionIdPart()));
}
break;
} }
// Populate Bundle.entry.search
String searchMode = ResourceMetadataKeyEnum.ENTRY_SEARCH_MODE.get(nextAsResource); String searchMode = ResourceMetadataKeyEnum.ENTRY_SEARCH_MODE.get(nextAsResource);
if (searchMode != null) { if (searchMode != null) {
entry.getSearch().getModeElement().setValueAsString(searchMode); entry.getSearch().getModeElement().setValueAsString(searchMode);

View File

@ -2,6 +2,8 @@ package ca.uhn.fhir.rest.server;
import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.model.api.Include;
import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum;
import ca.uhn.fhir.model.valueset.BundleEntrySearchModeEnum;
import ca.uhn.fhir.rest.annotation.IncludeParam; import ca.uhn.fhir.rest.annotation.IncludeParam;
import ca.uhn.fhir.rest.annotation.RequiredParam; import ca.uhn.fhir.rest.annotation.RequiredParam;
import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.annotation.Search;
@ -15,6 +17,8 @@ import ca.uhn.fhir.rest.param.TokenAndListParam;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.util.TestUtil; import ca.uhn.fhir.util.TestUtil;
import ca.uhn.fhir.util.UrlUtil; import ca.uhn.fhir.util.UrlUtil;
import ca.uhn.fhir.validation.FhirValidator;
import ca.uhn.fhir.validation.ValidationResult;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import org.apache.commons.io.IOUtils; import org.apache.commons.io.IOUtils;
import org.apache.http.client.ClientProtocolException; import org.apache.http.client.ClientProtocolException;
@ -28,6 +32,7 @@ import org.eclipse.jetty.servlet.ServletHandler;
import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.servlet.ServletHolder;
import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.r4.model.*; import org.hl7.fhir.r4.model.*;
import org.hl7.fhir.r4.utils.IResourceValidator;
import org.junit.AfterClass; import org.junit.AfterClass;
import org.junit.Before; import org.junit.Before;
import org.junit.BeforeClass; import org.junit.BeforeClass;
@ -64,20 +69,18 @@ public class SearchR4Test {
} }
private Bundle executeAndReturnLinkNext(HttpGet httpGet, EncodingEnum theExpectEncoding) throws IOException, ClientProtocolException { private Bundle executeAndReturnLinkNext(HttpGet httpGet, EncodingEnum theExpectEncoding) throws IOException, ClientProtocolException {
CloseableHttpResponse status = ourClient.execute(httpGet);
Bundle bundle; Bundle bundle;
try { try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
ourLog.info(responseContent); ourLog.info(responseContent);
assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals(200, status.getStatusLine().getStatusCode());
EncodingEnum ct = EncodingEnum.forContentType(status.getEntity().getContentType().getValue().replaceAll(";.*", "").trim()); EncodingEnum ct = EncodingEnum.forContentType(status.getEntity().getContentType().getValue().replaceAll(";.*", "").trim());
assertEquals(theExpectEncoding, ct); assertEquals(theExpectEncoding, ct);
bundle = ct.newParser(ourCtx).parseResource(Bundle.class, responseContent); bundle = ct.newParser(ourCtx).parseResource(Bundle.class, responseContent);
validate(bundle);
assertEquals(10, bundle.getEntry().size()); assertEquals(10, bundle.getEntry().size());
String linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl(); String linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl();
assertNotNull(linkNext); assertNotNull(linkNext);
} finally {
IOUtils.closeQuietly(status.getEntity().getContent());
} }
return bundle; return bundle;
} }
@ -346,19 +349,17 @@ public class SearchR4Test {
@Test @Test
public void testSearchNormal() throws Exception { public void testSearchNormal() throws Exception {
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar"); HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar&_pretty=true");
CloseableHttpResponse status = ourClient.execute(httpGet); try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
try {
String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
ourLog.info(responseContent); ourLog.info(responseContent);
validate(ourCtx.newJsonParser().parseResource(responseContent));
assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals(200, status.getStatusLine().getStatusCode());
assertEquals("search", ourLastMethod); assertEquals("search", ourLastMethod);
assertEquals("foo", ourIdentifiers.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getSystem()); assertEquals("foo", ourIdentifiers.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getSystem());
assertEquals("bar", ourIdentifiers.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue()); assertEquals("bar", ourIdentifiers.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue());
} finally {
IOUtils.closeQuietly(status.getEntity().getContent());
} }
} }
@ -366,8 +367,7 @@ public class SearchR4Test {
@Test @Test
public void testSearchWithInvalidChain() throws Exception { public void testSearchWithInvalidChain() throws Exception {
HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier.chain=foo%7Cbar"); HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier.chain=foo%7Cbar");
CloseableHttpResponse status = ourClient.execute(httpGet); try (CloseableHttpResponse status = ourClient.execute(httpGet)) {
try {
String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8);
ourLog.info(responseContent); ourLog.info(responseContent);
assertEquals(400, status.getStatusLine().getStatusCode()); assertEquals(400, status.getStatusLine().getStatusCode());
@ -376,14 +376,12 @@ public class SearchR4Test {
assertEquals( assertEquals(
"Invalid search parameter \"identifier.chain\". Parameter contains a chain (.chain) and chains are not supported for this parameter (chaining is only allowed on reference parameters)", "Invalid search parameter \"identifier.chain\". Parameter contains a chain (.chain) and chains are not supported for this parameter (chaining is only allowed on reference parameters)",
oo.getIssueFirstRep().getDiagnostics()); oo.getIssueFirstRep().getDiagnostics());
} finally {
IOUtils.closeQuietly(status.getEntity().getContent());
} }
} }
@Test @Test
public void testSearchWithPostAndInvalidParameters() throws Exception { public void testSearchWithPostAndInvalidParameters() {
IGenericClient client = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort); IGenericClient client = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort);
LoggingInterceptor interceptor = new LoggingInterceptor(); LoggingInterceptor interceptor = new LoggingInterceptor();
interceptor.setLogRequestSummary(true); interceptor.setLogRequestSummary(true);
@ -414,6 +412,15 @@ public class SearchR4Test {
return ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(theBundle); return ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(theBundle);
} }
protected void validate(IBaseResource theResource) {
FhirValidator validatorModule = ourCtx.newValidator();
ValidationResult result = validatorModule.validateWithResult(theResource);
if (!result.isSuccessful()) {
fail(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(result.toOperationOutcome()));
}
}
public static class DummyPatientResourceProvider implements IResourceProvider { public static class DummyPatientResourceProvider implements IResourceProvider {
@Override @Override
@ -427,13 +434,14 @@ public class SearchR4Test {
@RequiredParam(name = Patient.SP_IDENTIFIER) TokenAndListParam theIdentifiers) { @RequiredParam(name = Patient.SP_IDENTIFIER) TokenAndListParam theIdentifiers) {
ourLastMethod = "search"; ourLastMethod = "search";
ourIdentifiers = theIdentifiers; ourIdentifiers = theIdentifiers;
ArrayList<Patient> retVal = new ArrayList<Patient>(); ArrayList<Patient> retVal = new ArrayList<>();
for (int i = 0; i < 200; i++) { for (int i = 0; i < 200; i++) {
Patient patient = new Patient(); Patient patient = new Patient();
patient.getIdElement().setValue("Patient/" + i + "/_history/222");
ResourceMetadataKeyEnum.ENTRY_SEARCH_MODE.put(patient, BundleEntrySearchModeEnum.INCLUDE.getCode());
patient.addName(new HumanName().setFamily("FAMILY")); patient.addName(new HumanName().setFamily("FAMILY"));
patient.setActive(true); patient.setActive(true);
patient.getIdElement().setValue("Patient/" + i);
retVal.add(patient); retVal.add(patient);
} }
return retVal; return retVal;

View File

@ -106,6 +106,12 @@
a resource in an entry that required a resource (e.g. a POST). Thanks to a resource in an entry that required a resource (e.g. a POST). Thanks to
GitHub user @lytvynenko-dmitriy for reporting! GitHub user @lytvynenko-dmitriy for reporting!
</action> </action>
<action type="fix" issue="1250">
HAPI FHIR Server (plain, JPA, and JAX-RS) all populated Bundle.entry.result
on search result bundles, even though the FHIR specification states that this
should not be populated. This has been corrected. Thanks to GitHub user
@gitrust for reporting!
</action>
</release> </release>
<release version="3.8.0" date="2019-05-30" description="Hippo"> <release version="3.8.0" date="2019-05-30" description="Hippo">
<action type="fix"> <action type="fix">