From c8a70c19043eb61ff91808e18d8c0c79df397c94 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 23 Mar 2015 07:55:32 +0100 Subject: [PATCH] Fix #124 - Resource references shouldn't include version when they are encoded --- .../java/ca/uhn/fhir/parser/BaseParser.java | 23 +++- .../main/java/ca/uhn/fhir/parser/IParser.java | 30 ++++- .../java/ca/uhn/fhir/parser/JsonParser.java | 6 +- .../fhir/rest/server/RestfulServerUtils.java | 9 +- .../uhn/fhir/jpa/dao/BaseFhirResourceDao.java | 1 - .../ca/uhn/fhir/parser/JsonParserTest.java | 20 ++++ .../ca/uhn/fhir/parser/XmlParserTest.java | 20 ++++ .../ca/uhn/fhir/rest/server/ReadTest.java | 15 +++ .../ca/uhn/fhir/rest/server/SearchTest.java | 24 ++++ .../uhn/fhir/rest/server/SearchDstu2Test.java | 105 ++++++++++++++++++ src/changes/changes.xml | 30 +++-- 11 files changed, 262 insertions(+), 21 deletions(-) create mode 100644 hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/SearchDstu2Test.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java index 7358b5556ad..2159f48d9ac 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/BaseParser.java @@ -21,6 +21,7 @@ package ca.uhn.fhir.parser; */ import static org.apache.commons.lang3.StringUtils.isBlank; +import static org.apache.commons.lang3.StringUtils.isNotBlank; import java.io.IOException; import java.io.Reader; @@ -65,6 +66,7 @@ public abstract class BaseParser implements IParser { private ContainedResources myContainedResources; private FhirContext myContext; private boolean mySuppressNarratives; + private String myServerBaseUrl; public BaseParser(FhirContext theContext) { myContext = theContext; @@ -163,8 +165,9 @@ public abstract class BaseParser implements IParser { } protected String determineReferenceText(BaseResourceReferenceDt theRef) { - String reference = theRef.getReference().getValue(); - if (isBlank(reference)) { + IdDt ref = theRef.getReference(); + if (isBlank(ref.getIdPart())) { + String reference = ref.getValue(); if (theRef.getResource() != null) { IdDt containedId = getContainedResources().getResourceId(theRef.getResource()); if (containedId != null && !containedId.isEmpty()) { @@ -177,8 +180,22 @@ public abstract class BaseParser implements IParser { reference = theRef.getResource().getId().getValue(); } } + return reference; + } else { + if (isNotBlank(myServerBaseUrl) && StringUtils.equals(myServerBaseUrl, ref.getBaseUrl())) { + String reference = ref.toUnqualifiedVersionless().getValue(); + return reference; + } else { + String reference = ref.toVersionless().getValue(); + return reference; + } } - return reference; + } + + @Override + public IParser setServerBaseUrl(String theUrl) { + myServerBaseUrl = isNotBlank(theUrl) ? theUrl : null; + return this; } protected String determineResourceBaseUrl(String bundleBaseUrl, BundleEntry theEntry) { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/IParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/IParser.java index c074fe0f146..1ca5680bab5 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/IParser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/IParser.java @@ -32,7 +32,8 @@ import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.TagList; /** - * + * A parser, which can be used to convert between HAPI FHIR model/structure objects, and + * their respective String wire formats, in either XML or JSON. *

* Thread safety: Parsers are not guaranteed to be thread safe. Create a new parser instance for every thread or every message being parsed/encoded. *

@@ -66,10 +67,25 @@ public interface IParser { */ void encodeTagListToWriter(TagList theTagList, Writer theWriter) throws IOException; + /** + * Parse a DSTU1 style Atom Bundle. Note that as of DSTU2, Bundle is a resource so you + * should use {@link #parseResource(Class, Reader)} with the Bundle class found in the + * ca.uhn.hapi.fhir.model.[version].resource package instead. + */ Bundle parseBundle(Class theResourceType, Reader theReader); + /** + * Parse a DSTU1 style Atom Bundle. Note that as of DSTU2, Bundle is a resource so you + * should use {@link #parseResource(Class, Reader)} with the Bundle class found in the + * ca.uhn.hapi.fhir.model.[version].resource package instead. + */ Bundle parseBundle(Reader theReader); + /** + * Parse a DSTU1 style Atom Bundle. Note that as of DSTU2, Bundle is a resource so you + * should use {@link #parseResource(Class, String)} with the Bundle class found in the + * ca.uhn.hapi.fhir.model.[version].resource package instead. + */ Bundle parseBundle(String theMessageString) throws ConfigurationException, DataFormatException; /** @@ -143,7 +159,7 @@ public interface IParser { * * @param thePrettyPrint * The flag - * @return Returns an instance of this parser so that method calls can be conveniently chained + * @return Returns an instance of this parser so that method calls can be chained together */ IParser setPrettyPrint(boolean thePrettyPrint); @@ -152,4 +168,14 @@ public interface IParser { */ IParser setSuppressNarratives(boolean theSuppressNarratives); + /** + * Sets the server's base URL used by this parser. If a value is set, resource references + * will be turned into relative references if they are provided as absolute URLs but + * have a base matching the given base. + * + * @param theUrl The base URL, e.g. "http://example.com/base" + * @return Returns an instance of this parser so that method calls can be chained together + */ + IParser setServerBaseUrl(String theUrl); + } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java index 4c75a96ae85..6a66c5c3e32 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java @@ -49,16 +49,13 @@ import javax.json.stream.JsonGenerator; import javax.json.stream.JsonGeneratorFactory; import javax.json.stream.JsonParsingException; -import ca.uhn.fhir.model.base.composite.BaseCodingDt; -import ca.uhn.fhir.model.primitive.*; - import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.IBase; import org.hl7.fhir.instance.model.IBaseResource; import org.hl7.fhir.instance.model.IPrimitiveType; -import org.hl7.fhir.instance.model.api.IBaseBinary; import org.hl7.fhir.instance.model.api.IAnyResource; +import org.hl7.fhir.instance.model.api.IBaseBinary; import org.hl7.fhir.instance.model.api.IBaseBooleanDatatype; import org.hl7.fhir.instance.model.api.IBaseDatatype; import org.hl7.fhir.instance.model.api.IBaseDecimalDatatype; @@ -91,6 +88,7 @@ import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.model.api.Tag; import ca.uhn.fhir.model.api.TagList; import ca.uhn.fhir.model.api.annotation.Child; +import ca.uhn.fhir.model.base.composite.BaseCodingDt; import ca.uhn.fhir.model.base.composite.BaseContainedDt; import ca.uhn.fhir.model.base.composite.BaseNarrativeDt; import ca.uhn.fhir.model.base.composite.BaseResourceReferenceDt; diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java index 45090229e7c..37a497e1130 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/RestfulServerUtils.java @@ -147,7 +147,9 @@ public class RestfulServerUtils { if (theNarrativeMode == RestfulServer.NarrativeModeEnum.ONLY) { writer.append(theResource.getText().getDiv().getValueAsString()); } else { - getNewParser(theServer.getFhirContext(), responseEncoding, thePrettyPrint, theNarrativeMode).encodeResourceToWriter(theResource, writer); + IParser parser = getNewParser(theServer.getFhirContext(), responseEncoding, thePrettyPrint, theNarrativeMode); + parser.setServerBaseUrl(theServerBase); + parser.encodeResourceToWriter(theResource, writer); } } finally { writer.close(); @@ -372,8 +374,9 @@ public class RestfulServerUtils { writer.append("
"); } } else { - IParser newParser = RestfulServerUtils.getNewParser(theServer.getFhirContext(), responseEncoding, thePrettyPrint, theNarrativeMode); - newParser.encodeBundleToWriter(bundle, writer); + IParser parser = RestfulServerUtils.getNewParser(theServer.getFhirContext(), responseEncoding, thePrettyPrint, theNarrativeMode); + parser.setServerBaseUrl(theServerBase); + parser.encodeBundleToWriter(bundle, writer); } } finally { writer.close(); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseFhirResourceDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseFhirResourceDao.java index 17b72835bd1..c620e81246e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseFhirResourceDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseFhirResourceDao.java @@ -1122,7 +1122,6 @@ public abstract class BaseFhirResourceDao extends BaseFhirD CriteriaBuilder builder = myEntityManager.getCriteriaBuilder(); CriteriaQuery cq = builder.createQuery(ResourceTable.class); Root from = cq.from(ResourceTable.class); -// cq.where(builder.equal(from.get("myResourceType"), getContext().getResourceDefinition(myResourceType).getName())); cq.where(from.get("myId").in(theIncludePids)); TypedQuery q = myEntityManager.createQuery(cq); diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java index 0e1f40d3c75..f1503066ac6 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java @@ -101,6 +101,26 @@ public class JsonParserTest { } + @Test + public void testEncodeOmitsVersionAndBase() { + Patient p = new Patient(); + p.getManagingOrganization().setReference("http://example.com/base/Patient/1/_history/2"); + + String enc; + + enc = ourCtx.newJsonParser().encodeResourceToString(p); + ourLog.info(enc); + assertThat(enc, containsString("\"http://example.com/base/Patient/1\"")); + + enc = ourCtx.newJsonParser().setServerBaseUrl("http://example.com/base").encodeResourceToString(p); + ourLog.info(enc); + assertThat(enc, containsString("\"Patient/1\"")); + + enc = ourCtx.newJsonParser().setServerBaseUrl("http://example.com/base2").encodeResourceToString(p); + ourLog.info(enc); + assertThat(enc, containsString("\"http://example.com/base/Patient/1\"")); + } + @Test public void testDecimalPrecisionPreserved() { String number = "52.3779939997090374535378485873776474764643249869328698436986235758587"; diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java index 9592db55a38..8591b53b8e2 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/XmlParserTest.java @@ -97,6 +97,26 @@ public class XmlParserTest { } + @Test + public void testEncodeOmitsVersionAndBase() { + Patient p = new Patient(); + p.getManagingOrganization().setReference("http://example.com/base/Patient/1/_history/2"); + + String enc; + + enc = ourCtx.newXmlParser().encodeResourceToString(p); + ourLog.info(enc); + assertThat(enc, containsString("\"http://example.com/base/Patient/1\"")); + + enc = ourCtx.newXmlParser().setServerBaseUrl("http://example.com/base").encodeResourceToString(p); + ourLog.info(enc); + assertThat(enc, containsString("\"Patient/1\"")); + + enc = ourCtx.newXmlParser().setServerBaseUrl("http://example.com/base2").encodeResourceToString(p); + ourLog.info(enc); + assertThat(enc, containsString("\"http://example.com/base/Patient/1\"")); + } + @Test public void testDuplicateContainedResources() { diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ReadTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ReadTest.java index ccf7117fd52..70b63010e84 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ReadTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ReadTest.java @@ -4,6 +4,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.stringContainsInOrder; import static org.junit.Assert.*; +import java.io.IOException; import java.net.URLEncoder; import java.util.concurrent.TimeUnit; @@ -67,6 +68,19 @@ public class ReadTest { assertThat(responseContent, stringContainsInOrder("1", "\"")); assertThat(responseContent, not(stringContainsInOrder("1", "\"", "1"))); } + + @Test + public void testEncodeConvertsReferencesToRelative() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1?_format=xml"); + HttpResponse status = ourClient.execute(httpGet); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + String ref = ourCtx.newXmlParser().parseResource(Patient.class, responseContent).getManagingOrganization().getReference().getValue(); + assertEquals("Organization/555", ref); + } @Test public void testReadJson() throws Exception { @@ -216,6 +230,7 @@ public class ReadTest { Patient patient = new Patient(); patient.addIdentifier(theId.getIdPart(), theId.getVersionIdPart()); patient.setId("Patient/1/_history/1"); + patient.getManagingOrganization().setReference("http://localhost:" + ourPort + "/Organization/555/_history/666"); return patient; } diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java index 5a967ce4821..943cc4f7e58 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/SearchTest.java @@ -41,6 +41,7 @@ import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; import ca.uhn.fhir.rest.annotation.IdParam; import ca.uhn.fhir.rest.annotation.OptionalParam; +import ca.uhn.fhir.rest.annotation.Read; import ca.uhn.fhir.rest.annotation.RequiredParam; import ca.uhn.fhir.rest.annotation.Search; import ca.uhn.fhir.rest.client.IGenericClient; @@ -63,6 +64,20 @@ public class SearchTest { private static Server ourServer; + @Test + public void testEncodeConvertsReferencesToRelative() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_query=searchWithRef"); + HttpResponse status = ourClient.execute(httpGet); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + Patient patient = (Patient) ourCtx.newXmlParser().parseBundle(responseContent).getEntries().get(0).getResource(); + String ref = patient.getManagingOrganization().getReference().getValue(); + assertEquals("Organization/555", ref); + } + @Test public void testOmitEmptyOptionalParam() throws Exception { HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_id="); @@ -312,6 +327,15 @@ public class SearchTest { retVal.add(patient); return retVal; } + + @Search(queryName="searchWithRef") + public Patient searchWithRef() { + Patient patient = new Patient(); + patient.setId("Patient/1/_history/1"); + patient.getManagingOrganization().setReference("http://localhost:" + ourPort + "/Organization/555/_history/666"); + return patient; + } + @Search public List findPatient(@RequiredParam(name = "_id") StringParam theParam) { diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/SearchDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/SearchDstu2Test.java new file mode 100644 index 00000000000..3976340b623 --- /dev/null +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/server/SearchDstu2Test.java @@ -0,0 +1,105 @@ +package ca.uhn.fhir.rest.server; + +import static org.junit.Assert.assertEquals; + +import java.util.concurrent.TimeUnit; + +import org.apache.commons.io.IOUtils; +import org.apache.http.HttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.api.IResource; +import ca.uhn.fhir.model.dstu2.resource.Bundle; +import ca.uhn.fhir.model.dstu2.resource.Patient; +import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator; +import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.util.PortUtil; + +/** + * Created by dsotnikov on 2/25/2014. + */ +public class SearchDstu2Test { + + private static CloseableHttpClient ourClient; + private static FhirContext ourCtx = new FhirContext(); + private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(SearchDstu2Test.class); + private static int ourPort; + + private static Server ourServer; + + @Test + public void testEncodeConvertsReferencesToRelative() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient?_query=searchWithRef"); + HttpResponse status = ourClient.execute(httpGet); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + ourLog.info(responseContent); + + assertEquals(200, status.getStatusLine().getStatusCode()); + Patient patient = (Patient) ourCtx.newXmlParser().parseResource(Bundle.class, responseContent).getEntry().get(0).getResource(); + String ref = patient.getManagingOrganization().getReference().getValue(); + assertEquals("Organization/555", ref); + } + + + @AfterClass + public static void afterClass() throws Exception { + ourServer.stop(); + } + + @BeforeClass + public static void beforeClass() throws Exception { + ourPort = PortUtil.findFreePort(); + ourServer = new Server(ourPort); + + DummyPatientResourceProvider patientProvider = new DummyPatientResourceProvider(); + + ServletHandler proxyHandler = new ServletHandler(); + RestfulServer servlet = new RestfulServer(); + servlet.getFhirContext().setNarrativeGenerator(new DefaultThymeleafNarrativeGenerator()); + + servlet.setResourceProviders(patientProvider); + ServletHolder servletHolder = new ServletHolder(servlet); + proxyHandler.addServletWithMapping(servletHolder, "/*"); + ourServer.setHandler(proxyHandler); + ourServer.start(); + + PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS); + HttpClientBuilder builder = HttpClientBuilder.create(); + builder.setConnectionManager(connectionManager); + ourClient = builder.build(); + + } + + + /** + * Created by dsotnikov on 2/25/2014. + */ + public static class DummyPatientResourceProvider implements IResourceProvider { + + @Search(queryName="searchWithRef") + public Patient searchWithRef() { + Patient patient = new Patient(); + patient.setId("Patient/1/_history/1"); + patient.getManagingOrganization().setReference("http://localhost:" + ourPort + "/Organization/555/_history/666"); + return patient; + } + + @Override + public Class getResourceType() { + return Patient.class; + } + + } + +} diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 037ca180a19..f78065d0eda 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -4,17 +4,22 @@ James Agnew HAPI FHIR Changelog - + + Bump the version of a few dependencies to the + latest versions: + +
  • Phloc-commons (for schematron validation) 4.3.5 -> 4.3.6
    • +
  • Apache HttpClient 4.3.6 -> 4.4
    • +
  • Woodstox 4.4.0 -> 4.4.1
    • +
  • SLF4j 1.7.9 -> 1.7.10
    • +
  • Spring (used in hapi-fhir-jpaserver-base module) 4.1.3.RELEASE -> 4.1.5.RELEASE
    • + + ]]> +     Add support for "profile" and "tag" elements in the resource Meta block when parsing DSTU2 structures. @@ -50,6 +55,15 @@ the new syntax required in DSTU2: [resource name]:[search param NAME] insead of the DSTU1 style [resource name].[search param PATH] + + When encoding resources, the parser will now convert any resource + references to versionless references automatically (i.e. it will + omit the version part automatically if one is present in the reference) + since references between resources must be versionless. Additionally, + references in server responses will omit the server base URL part of the + reference if the base matches the base for the server giving + the response. +