From 638cb976fd1d076c27d31152e77c33ca10cfb453 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Sat, 22 Jun 2019 16:23:32 -0400 Subject: [PATCH] Fix #1250 - Don't include Bundle.entry.response on search results --- .../fhir/model/valueset/BundleTypeEnum.java | 4 +- .../hapi/rest/server/Dstu3BundleFactory.java | 37 ++++++++++++----- .../r4/hapi/rest/server/R4BundleFactory.java | 23 +++++++---- .../ca/uhn/fhir/rest/server/SearchR4Test.java | 40 +++++++++++-------- src/changes/changes.xml | 6 +++ 5 files changed, 76 insertions(+), 34 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/valueset/BundleTypeEnum.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/valueset/BundleTypeEnum.java index 7f0ab553ba8..a41678970bf 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/valueset/BundleTypeEnum.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/valueset/BundleTypeEnum.java @@ -35,7 +35,9 @@ public enum BundleTypeEnum { DOCUMENT("document", "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"), HISTORY("history", "http://hl7.org/fhir/bundle-type"), diff --git a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/Dstu3BundleFactory.java b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/Dstu3BundleFactory.java index 85a084e574a..97b70b81820 100644 --- a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/Dstu3BundleFactory.java +++ b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/rest/server/Dstu3BundleFactory.java @@ -9,9 +9,9 @@ package org.hl7.fhir.dstu3.hapi.rest.server; * 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. @@ -27,6 +27,7 @@ import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.model.valueset.BundleTypeEnum; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.IVersionSpecificBundleFactory; +import ca.uhn.fhir.rest.server.RestfulServerUtils; import ca.uhn.fhir.util.ResourceReferenceInfo; import org.hl7.fhir.dstu3.model.Bundle; 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) { BundleEntryComponent entry = myBundle.addEntry(); entry.setResource((Resource) next).getSearch().setMode(SearchEntryMode.INCLUDE); @@ -199,7 +200,7 @@ public class Dstu3BundleFactory implements IVersionSpecificBundleFactory { includedResources.addAll(addedResourcesThisPass); // Linked resources may themselves have linked resources - references = new ArrayList(); + references = new ArrayList<>(); for (IAnyResource iResource : addedResourcesThisPass) { List newReferences = myContext.newTerser().getAllResourceReferences(iResource); references.addAll(newReferences); @@ -220,15 +221,33 @@ public class Dstu3BundleFactory implements IVersionSpecificBundleFactory { 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); if (searchMode != null) { entry.getSearch().getModeElement().setValueAsString(searchMode); } + } - /* - * Actually add the resources to the bundle - */ + /* + * Actually add the resources to the bundle + */ for (IAnyResource next : includedResources) { BundleEntryComponent entry = myBundle.addEntry(); entry.setResource((Resource) next).getSearch().setMode(SearchEntryMode.INCLUDE); diff --git a/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/rest/server/R4BundleFactory.java b/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/rest/server/R4BundleFactory.java index 240adc96065..34068b284f4 100644 --- a/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/rest/server/R4BundleFactory.java +++ b/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/rest/server/R4BundleFactory.java @@ -223,16 +223,23 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { entry.setResource(null); } - // Populate Response - 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())); + // 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); if (searchMode != null) { entry.getSearch().getModeElement().setValueAsString(searchMode); diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchR4Test.java index 0ab9ece8c3b..0a128d3dac2 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/SearchR4Test.java @@ -2,6 +2,8 @@ package ca.uhn.fhir.rest.server; import ca.uhn.fhir.context.FhirContext; 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.RequiredParam; 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.util.TestUtil; 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 org.apache.commons.io.IOUtils; import org.apache.http.client.ClientProtocolException; @@ -28,6 +32,7 @@ import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.model.*; +import org.hl7.fhir.r4.utils.IResourceValidator; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; @@ -64,20 +69,18 @@ public class SearchR4Test { } private Bundle executeAndReturnLinkNext(HttpGet httpGet, EncodingEnum theExpectEncoding) throws IOException, ClientProtocolException { - CloseableHttpResponse status = ourClient.execute(httpGet); Bundle bundle; - try { + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(responseContent); assertEquals(200, status.getStatusLine().getStatusCode()); EncodingEnum ct = EncodingEnum.forContentType(status.getEntity().getContentType().getValue().replaceAll(";.*", "").trim()); assertEquals(theExpectEncoding, ct); bundle = ct.newParser(ourCtx).parseResource(Bundle.class, responseContent); + validate(bundle); assertEquals(10, bundle.getEntry().size()); String linkNext = bundle.getLink(Constants.LINK_NEXT).getUrl(); assertNotNull(linkNext); - } finally { - IOUtils.closeQuietly(status.getEntity().getContent()); } return bundle; } @@ -346,19 +349,17 @@ public class SearchR4Test { @Test public void testSearchNormal() throws Exception { - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar"); - CloseableHttpResponse status = ourClient.execute(httpGet); - try { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier=foo%7Cbar&_pretty=true"); + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(responseContent); + validate(ourCtx.newJsonParser().parseResource(responseContent)); assertEquals(200, status.getStatusLine().getStatusCode()); assertEquals("search", ourLastMethod); assertEquals("foo", ourIdentifiers.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getSystem()); assertEquals("bar", ourIdentifiers.getValuesAsQueryTokens().get(0).getValuesAsQueryTokens().get(0).getValue()); - } finally { - IOUtils.closeQuietly(status.getEntity().getContent()); } } @@ -366,8 +367,7 @@ public class SearchR4Test { @Test public void testSearchWithInvalidChain() throws Exception { HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?identifier.chain=foo%7Cbar"); - CloseableHttpResponse status = ourClient.execute(httpGet); - try { + try (CloseableHttpResponse status = ourClient.execute(httpGet)) { String responseContent = IOUtils.toString(status.getEntity().getContent(), StandardCharsets.UTF_8); ourLog.info(responseContent); assertEquals(400, status.getStatusLine().getStatusCode()); @@ -376,14 +376,12 @@ public class SearchR4Test { 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)", oo.getIssueFirstRep().getDiagnostics()); - } finally { - IOUtils.closeQuietly(status.getEntity().getContent()); } } @Test - public void testSearchWithPostAndInvalidParameters() throws Exception { + public void testSearchWithPostAndInvalidParameters() { IGenericClient client = ourCtx.newRestfulGenericClient("http://localhost:" + ourPort); LoggingInterceptor interceptor = new LoggingInterceptor(); interceptor.setLogRequestSummary(true); @@ -414,6 +412,15 @@ public class SearchR4Test { 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 { @Override @@ -427,13 +434,14 @@ public class SearchR4Test { @RequiredParam(name = Patient.SP_IDENTIFIER) TokenAndListParam theIdentifiers) { ourLastMethod = "search"; ourIdentifiers = theIdentifiers; - ArrayList retVal = new ArrayList(); + ArrayList retVal = new ArrayList<>(); for (int i = 0; i < 200; i++) { 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.setActive(true); - patient.getIdElement().setValue("Patient/" + i); retVal.add(patient); } return retVal; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 707e9722ba0..75a9c96090e 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -106,6 +106,12 @@ a resource in an entry that required a resource (e.g. a POST). Thanks to GitHub user @lytvynenko-dmitriy for reporting! + + 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! +