From 5a2ee7714248f634259ba25b15434fd693f8431d Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Mon, 26 Oct 2015 22:47:06 -0400 Subject: [PATCH] Parser (XML and JSON) shouldn't encode an ID tag in resources which are part of a bundle when the resource has a UUID/OID ID. --- .../java/example/TransactionClientTest.java | 64 ++++++ .../java/ca/uhn/fhir/parser/JsonParser.java | 34 ++- .../java/ca/uhn/fhir/parser/XmlParser.java | 30 ++- .../uhn/fhir/rest/server/ReadDstu1Test.java | 210 +++++++++--------- .../uhn/fhir/parser/JsonParserDstu2Test.java | 13 +- .../uhn/fhir/parser/XmlParserDstu2Test.java | 14 +- src/changes/changes.xml | 5 + 7 files changed, 240 insertions(+), 130 deletions(-) create mode 100644 examples/src/main/java/example/TransactionClientTest.java diff --git a/examples/src/main/java/example/TransactionClientTest.java b/examples/src/main/java/example/TransactionClientTest.java new file mode 100644 index 00000000000..9f989efb2a3 --- /dev/null +++ b/examples/src/main/java/example/TransactionClientTest.java @@ -0,0 +1,64 @@ +package example; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.dstu2.composite.QuantityDt; +import ca.uhn.fhir.model.dstu2.composite.ResourceReferenceDt; +import ca.uhn.fhir.model.dstu2.resource.Bundle; +import ca.uhn.fhir.model.dstu2.resource.Observation; +import ca.uhn.fhir.model.dstu2.resource.Patient; +import ca.uhn.fhir.model.dstu2.valueset.AdministrativeGenderEnum; +import ca.uhn.fhir.model.dstu2.valueset.BundleTypeEnum; +import ca.uhn.fhir.model.dstu2.valueset.ObservationStatusEnum; +import ca.uhn.fhir.model.primitive.IdDt; + +public class TransactionClientTest { + + public static void main(String[] args) { + conditionalCreate(); + } + + private static void conditionalCreate() { + + // Create a patient object + Patient patient = new Patient(); + patient + .addIdentifier() + .setSystem("http://acme.org/mrns") + .setValue("12345"); + patient + .addName() + .addFamily("Jones") + .addGiven("Johnson"); + patient.setGender(AdministrativeGenderEnum.MALE); + + // Create an observation object + Observation observation = new Observation(); + observation.setStatus(ObservationStatusEnum.FINAL); + observation.setSubject(new ResourceReferenceDt(patient)); + observation + .getCode() + .addCoding() + .setSystem("http://loinc.org") + .setCode("789-8") + .setDisplay("Erythrocytes [#/volume] in Blood by Automated count"); + observation.setValue( + new QuantityDt() + .setValue(4.12) + .setUnit("10 trillion/L") + .setSystem("http://unitsofmeasure.org") + .setCode("10*12/L")); + + Bundle bundle = new Bundle(); + bundle.setType(BundleTypeEnum.TRANSACTION); + + patient.setId(IdDt.newRandomUuid()); + bundle + .addEntry() + .setResource(patient) + .getRequest() + .setUrl(patient.getId().getValue()); + + System.out.println(FhirContext.forDstu2().newXmlParser().setPrettyPrint(true).encodeResourceToString(bundle)); + } + +} 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 d1b924dcccd..c29676a95bc 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 @@ -698,19 +698,29 @@ public class JsonParser extends BaseParser implements IParser { private void encodeResourceToJsonStreamWriter(RuntimeResourceDefinition theResDef, IBaseResource theResource, JsonGenerator theEventWriter, String theObjectNameOrNull, boolean theContainedResource) throws IOException { String resourceId = null; - if (theResource instanceof IResource) { - IResource res = (IResource) theResource; - if (StringUtils.isNotBlank(res.getId().getIdPart())) { - if (theContainedResource) { - resourceId = res.getId().getIdPart(); - } else if (myContext.getVersion().getVersion().isNewerThan(FhirVersionEnum.DSTU1)) { - resourceId = res.getId().getIdPart(); - } +// if (theResource instanceof IResource) { +// IResource res = (IResource) theResource; +// if (StringUtils.isNotBlank(res.getId().getIdPart())) { +// if (theContainedResource) { +// resourceId = res.getId().getIdPart(); +// } else if (myContext.getVersion().getVersion().isNewerThan(FhirVersionEnum.DSTU1)) { +// resourceId = res.getId().getIdPart(); +// } +// } +// } else if (theResource instanceof IAnyResource) { +// IAnyResource res = (IAnyResource) theResource; +// if (/* theContainedResource && */StringUtils.isNotBlank(res.getIdElement().getIdPart())) { +// resourceId = res.getIdElement().getIdPart(); +// } +// } + + if (StringUtils.isNotBlank(theResource.getIdElement().getIdPart())) { + resourceId = theResource.getIdElement().getIdPart(); + if (theResource.getIdElement().getValue().startsWith("urn:")) { + resourceId = null; } - } else if (theResource instanceof IAnyResource) { - IAnyResource res = (IAnyResource) theResource; - if (/* theContainedResource && */StringUtils.isNotBlank(res.getIdElement().getIdPart())) { - resourceId = res.getIdElement().getIdPart(); + if (myContext.getVersion().getVersion().equals(FhirVersionEnum.DSTU1)) { + resourceId = null; } } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java index e7f4581f0ca..5cf70ca0d32 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java @@ -716,17 +716,27 @@ public class XmlParser extends BaseParser implements IParser { private void encodeResourceToXmlStreamWriter(IBaseResource theResource, XMLStreamWriter theEventWriter, boolean theIncludedResource) throws XMLStreamException, DataFormatException { String resourceId = null; - if (theResource instanceof IResource) { - // HAPI structs - IResource iResource = (IResource) theResource; - if (StringUtils.isNotBlank(iResource.getId().getIdPart())) { - resourceId = iResource.getId().getIdPart(); +// if (theResource instanceof IResource) { +// // HAPI structs +// IResource iResource = (IResource) theResource; +// if (StringUtils.isNotBlank(iResource.getId().getIdPart())) { +// resourceId = iResource.getId().getIdPart(); +// } +// } else { +// // HL7 structs +// IAnyResource resource = (IAnyResource) theResource; +// if (StringUtils.isNotBlank(resource.getIdElement().getIdPart())) { +// resourceId = resource.getIdElement().getIdPart(); +// } +// } + + if (StringUtils.isNotBlank(theResource.getIdElement().getIdPart())) { + resourceId = theResource.getIdElement().getIdPart(); + if (theResource.getIdElement().getValue().startsWith("urn:")) { + resourceId = null; } - } else { - // HL7 structs - IAnyResource resource = (IAnyResource) theResource; - if (StringUtils.isNotBlank(resource.getIdElement().getIdPart())) { - resourceId = resource.getIdElement().getIdPart(); + if (myContext.getVersion().getVersion().equals(FhirVersionEnum.DSTU1)) { + resourceId = null; } } diff --git a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ReadDstu1Test.java b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ReadDstu1Test.java index 6392fb9265a..44e58c677b8 100644 --- a/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ReadDstu1Test.java +++ b/hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/rest/server/ReadDstu1Test.java @@ -38,89 +38,10 @@ import ca.uhn.fhir.util.PortUtil; public class ReadDstu1Test { private static CloseableHttpClient ourClient; - private static int ourPort; - private static Server ourServer; private static final FhirContext ourCtx = FhirContext.forDstu1(); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ReadDstu1Test.class); - - @Test - public void testReadXml() 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()); - IdentifierDt dt = ourCtx.newXmlParser().parseResource(Patient.class, responseContent).getIdentifierFirstRep(); - - assertEquals("1", dt.getSystem().getValueAsString()); - assertEquals(null, dt.getValue().getValueAsString()); - - Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC); - assertNotNull(cl); - assertEquals("http://localhost:" + ourPort + "/Patient/1/_history/1", cl.getValue()); - - 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 { - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1?_format=json"); - 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()); - IdentifierDt dt = ourCtx.newJsonParser().parseResource(Patient.class, responseContent).getIdentifierFirstRep(); - - assertEquals("1", dt.getSystem().getValueAsString()); - assertEquals(null, dt.getValue().getValueAsString()); - - Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC); - assertNotNull(cl); - assertEquals("http://localhost:" + ourPort + "/Patient/1/_history/1", cl.getValue()); - - assertThat(responseContent, stringContainsInOrder("1", "\"")); - assertThat(responseContent, not(stringContainsInOrder("1", "\"", "1"))); - } - - @Test - public void testReadForProviderWithAbstractReturnType() throws Exception { - { - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Organization/1"); - HttpResponse status = ourClient.execute(httpGet); - String responseContent = IOUtils.toString(status.getEntity().getContent()); - IOUtils.closeQuietly(status.getEntity().getContent()); - - assertEquals(200, status.getStatusLine().getStatusCode()); - IdentifierDt dt = ourCtx.newXmlParser().parseResource(Organization.class, responseContent).getIdentifierFirstRep(); - - assertEquals("1", dt.getSystem().getValueAsString()); - assertEquals(null, dt.getValue().getValueAsString()); - - Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC); - assertNotNull(cl); - assertEquals("http://localhost:" + ourPort + "/Organization/1/_history/1", cl.getValue()); - - } - - } + private static int ourPort; + private static Server ourServer; @Test public void testBinaryRead() throws Exception { @@ -151,22 +72,60 @@ public class ReadDstu1Test { } @Test - public void testVRead() throws Exception { + 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 testReadForProviderWithAbstractReturnType() throws Exception { { - HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/_history/2"); + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Organization/1"); HttpResponse status = ourClient.execute(httpGet); String responseContent = IOUtils.toString(status.getEntity().getContent()); IOUtils.closeQuietly(status.getEntity().getContent()); assertEquals(200, status.getStatusLine().getStatusCode()); - IdentifierDt dt = ourCtx.newXmlParser().parseResource(Patient.class, responseContent).getIdentifierFirstRep(); + IdentifierDt dt = ourCtx.newXmlParser().parseResource(Organization.class, responseContent).getIdentifierFirstRep(); + assertEquals("1", dt.getSystem().getValueAsString()); - assertEquals("2", dt.getValue().getValueAsString()); + assertEquals(null, dt.getValue().getValueAsString()); Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC); assertNotNull(cl); - assertEquals("http://localhost:" + ourPort + "/Patient/1/_history/1", cl.getValue()); + assertEquals("http://localhost:" + ourPort + "/Organization/1/_history/1", cl.getValue()); + } + + } + + @Test + public void testReadJson() throws Exception { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1?_format=json"); + 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()); + IdentifierDt dt = ourCtx.newJsonParser().parseResource(Patient.class, responseContent).getIdentifierFirstRep(); + + assertEquals("1", dt.getSystem().getValueAsString()); + assertEquals(null, dt.getValue().getValueAsString()); + + Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC); + assertNotNull(cl); + assertEquals("http://localhost:" + ourPort + "/Patient/1/_history/1", cl.getValue()); + + assertThat(responseContent, stringContainsInOrder("1", "\"")); + assertThat(responseContent, not(stringContainsInOrder("1", "\"", "1"))); } @Test @@ -188,6 +147,47 @@ public class ReadDstu1Test { assertEquals(vid, dt.getValue().getValueAsString()); } + @Test + public void testReadXml() 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()); + IdentifierDt dt = ourCtx.newXmlParser().parseResource(Patient.class, responseContent).getIdentifierFirstRep(); + + assertEquals("1", dt.getSystem().getValueAsString()); + assertEquals(null, dt.getValue().getValueAsString()); + + Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC); + assertNotNull(cl); + assertEquals("http://localhost:" + ourPort + "/Patient/1/_history/1", cl.getValue()); + + assertThat(responseContent, stringContainsInOrder("1", "\"")); + assertThat(responseContent, not(stringContainsInOrder("1", "\"", "1"))); + } + + @Test + public void testVRead() throws Exception { + { + HttpGet httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/_history/2"); + HttpResponse status = ourClient.execute(httpGet); + String responseContent = IOUtils.toString(status.getEntity().getContent()); + IOUtils.closeQuietly(status.getEntity().getContent()); + + assertEquals(200, status.getStatusLine().getStatusCode()); + IdentifierDt dt = ourCtx.newXmlParser().parseResource(Patient.class, responseContent).getIdentifierFirstRep(); + assertEquals("1", dt.getSystem().getValueAsString()); + assertEquals("2", dt.getValue().getValueAsString()); + + Header cl = status.getFirstHeader(Constants.HEADER_CONTENT_LOCATION_LC); + assertNotNull(cl); + assertEquals("http://localhost:" + ourPort + "/Patient/1/_history/1", cl.getValue()); + } + } + @AfterClass public static void afterClass() throws Exception { ourServer.stop(); @@ -219,20 +219,20 @@ public class ReadDstu1Test { /** * Created by dsotnikov on 2/25/2014. */ - public static class PatientProvider implements IResourceProvider { + public static class BinaryProvider implements IResourceProvider { @Read(version = true) - public Patient read(@IdParam IdDt theId) { - 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; + public Binary findPatient(@IdParam IdDt theId) { + Binary bin = new Binary(); + bin.setContentType("application/x-foo"); + bin.setContent(new byte[] { 1, 2, 3, 4 }); + bin.setId("Binary/1/_history/1"); + return bin; } @Override public Class getResourceType() { - return Patient.class; + return Binary.class; } } @@ -260,20 +260,20 @@ public class ReadDstu1Test { /** * Created by dsotnikov on 2/25/2014. */ - public static class BinaryProvider implements IResourceProvider { - - @Read(version = true) - public Binary findPatient(@IdParam IdDt theId) { - Binary bin = new Binary(); - bin.setContentType("application/x-foo"); - bin.setContent(new byte[] { 1, 2, 3, 4 }); - bin.setId("Binary/1/_history/1"); - return bin; - } + public static class PatientProvider implements IResourceProvider { @Override public Class getResourceType() { - return Binary.class; + return Patient.class; + } + + @Read(version = true) + public Patient read(@IdParam IdDt theId) { + 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-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java index 3039780664f..a73d0cef4c1 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserDstu2Test.java @@ -66,7 +66,16 @@ public class JsonParserDstu2Test { assertThat(encoded, containsString("\"div\":\"" + expected.replace("\"", "\\\"") + "\"")); } - + @Test + public void testEncodeDoesntIncludeUuidId() { + Patient p = new Patient(); + p.setId(new IdDt("urn:uuid:42795ed8-041f-4ebf-b6f4-78ef6f64c2f2")); + p.addIdentifier().setSystem("ACME"); + + String actual = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(p); + assertThat(actual, not(containsString("78ef6f64c2f2"))); + } + @Test public void testEncodeEmptyBinary() { String output = ourCtx.newJsonParser().encodeResourceToString(new Binary()); @@ -773,7 +782,7 @@ public class JsonParserDstu2Test { assertEquals("urn:uuid:180f219f-97a8-486d-99d9-ed631fe4fc57", parsed.getEntry().get(0).getResource().getId().getValue()); assertEquals("urn:uuid:", parsed.getEntry().get(0).getResource().getId().getBaseUrl()); assertEquals("180f219f-97a8-486d-99d9-ed631fe4fc57", parsed.getEntry().get(0).getResource().getId().getIdPart()); - assertThat(encoded, containsString("\"id\":\"180f219f-97a8-486d-99d9-ed631fe4fc57\"")); + assertThat(encoded, not(containsString("\"id\":\"180f219f-97a8-486d-99d9-ed631fe4fc57\""))); } @Test diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java index 1d156a7572a..c3d6841c24b 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/parser/XmlParserDstu2Test.java @@ -131,6 +131,17 @@ public class XmlParserDstu2Test { } + @Test + public void testEncodeDoesntIncludeUuidId() { + Patient p = new Patient(); + p.setId(new IdDt("urn:uuid:42795ed8-041f-4ebf-b6f4-78ef6f64c2f2")); + p.addIdentifier().setSystem("ACME"); + + String actual = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(p); + assertThat(actual, not(containsString("78ef6f64c2f2"))); + } + + @Test public void testContainedResourceInExtensionUndeclared() { Patient p = new Patient(); @@ -661,7 +672,8 @@ public class XmlParserDstu2Test { String encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeBundleToString(b); ourLog.info(encoded); - assertThat(encoded, stringContainsInOrder("", "", "")); + assertThat(encoded, stringContainsInOrder("", "", ""))); } @Test diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 4fa882f66e3..e179f6a2667 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -207,6 +207,11 @@ FIx issue in testpage-overlay's new Java configuration where only the first configured server actually gets used. + + Parser (XML and JSON) shouldn't encode an ID tag in resources + which are part of a bundle when the resource has a UUID/OID + ID. +