From f92c5a723242b2dc42713e06ca7a38db5cf1c222 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 16 Sep 2016 15:01:14 -0400 Subject: [PATCH] Fix #446 - Don't give child resources the ID of the parent --- .../java/ca/uhn/fhir/parser/JsonParser.java | 12 +-- .../java/ca/uhn/fhir/parser/XmlParser.java | 12 +-- .../rest/client/GenericClientDstu3Test.java | 89 +++++++++++++++++++ src/changes/changes.xml | 6 ++ 4 files changed, 107 insertions(+), 12 deletions(-) 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 0f2735fe022..e5eb5336034 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 @@ -205,7 +205,7 @@ public class JsonParser extends BaseParser implements IParser { JsonWriter eventWriter = createJsonWriter(theWriter); RuntimeResourceDefinition resDef = myContext.getResourceDefinition(theResource); - encodeResourceToJsonStreamWriter(resDef, theResource, eventWriter, null, false); + encodeResourceToJsonStreamWriter(resDef, theResource, eventWriter, null, false, false); eventWriter.flush(); } @@ -320,7 +320,7 @@ public class JsonParser extends BaseParser implements IParser { IResource resource = nextEntry.getResource(); if (resource != null && !resource.isEmpty() && !deleted) { RuntimeResourceDefinition resDef = myContext.getResourceDefinition(resource); - encodeResourceToJsonStreamWriter(resDef, resource, theEventWriter, "content", false); + encodeResourceToJsonStreamWriter(resDef, resource, theEventWriter, "content", false, true); } if (nextEntry.getSummary().isEmpty() == false) { @@ -374,7 +374,7 @@ public class JsonParser extends BaseParser implements IParser { IResource resource = nextEntry.getResource(); if (resource != null && !resource.isEmpty() && !deleted) { RuntimeResourceDefinition resDef = myContext.getResourceDefinition(resource); - encodeResourceToJsonStreamWriter(resDef, resource, theEventWriter, "resource", false); + encodeResourceToJsonStreamWriter(resDef, resource, theEventWriter, "resource", false, true); } if (nextEntry.getSearchMode().isEmpty() == false || nextEntry.getScore().isEmpty() == false) { @@ -551,7 +551,7 @@ public class JsonParser extends BaseParser implements IParser { case RESOURCE: IBaseResource resource = (IBaseResource) theNextValue; RuntimeResourceDefinition def = myContext.getResourceDefinition(resource); - encodeResourceToJsonStreamWriter(def, resource, theEventWriter, theChildName, false); + encodeResourceToJsonStreamWriter(def, resource, theEventWriter, theChildName, false, true); break; case UNDECL_EXT: default: @@ -812,7 +812,7 @@ public class JsonParser extends BaseParser implements IParser { encodeCompositeElementChildrenToStreamWriter(theResDef, theResource, theNextValue, theEventWriter, theContainedResource, theParent); } - private void encodeResourceToJsonStreamWriter(RuntimeResourceDefinition theResDef, IBaseResource theResource, JsonWriter theEventWriter, String theObjectNameOrNull, boolean theContainedResource) throws IOException { + private void encodeResourceToJsonStreamWriter(RuntimeResourceDefinition theResDef, IBaseResource theResource, JsonWriter theEventWriter, String theObjectNameOrNull, boolean theContainedResource, boolean theSubResource) throws IOException { IIdType resourceId = null; // if (theResource instanceof IResource) { // IResource res = (IResource) theResource; @@ -843,7 +843,7 @@ public class JsonParser extends BaseParser implements IParser { if (!theContainedResource) { if (super.shouldEncodeResourceId(theResource) == false) { resourceId = null; - } else if (getEncodeForceResourceId() != null) { + } else if (!theSubResource && getEncodeForceResourceId() != null) { resourceId = getEncodeForceResourceId(); } } 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 0be6e2fc2e3..2468f079646 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 @@ -174,7 +174,7 @@ public class XmlParser extends BaseParser implements IParser { try { eventWriter = createXmlWriter(theWriter); - encodeResourceToXmlStreamWriter(theResource, eventWriter, false); + encodeResourceToXmlStreamWriter(theResource, eventWriter, false, false); eventWriter.flush(); } catch (XMLStreamException e) { throw new ConfigurationException("Failed to initialize STaX event factory", e); @@ -384,7 +384,7 @@ public class XmlParser extends BaseParser implements IParser { if (resource != null && !resource.isEmpty() && !deleted) { eventWriter.writeStartElement("content"); eventWriter.writeAttribute("type", "text/xml"); - encodeResourceToXmlStreamWriter(resource, eventWriter, false); + encodeResourceToXmlStreamWriter(resource, eventWriter, false, true); eventWriter.writeEndElement(); // content } else { ourLog.debug("Bundle entry contains null resource"); @@ -447,7 +447,7 @@ public class XmlParser extends BaseParser implements IParser { IResource resource = nextEntry.getResource(); if (resource != null && !resource.isEmpty() && !deleted) { theEventWriter.writeStartElement("resource"); - encodeResourceToXmlStreamWriter(resource, theEventWriter, false); + encodeResourceToXmlStreamWriter(resource, theEventWriter, false, true); theEventWriter.writeEndElement(); // content } else { ourLog.debug("Bundle entry contains null resource"); @@ -557,7 +557,7 @@ public class XmlParser extends BaseParser implements IParser { case RESOURCE: { theEventWriter.writeStartElement(childName); IBaseResource resource = (IBaseResource) theElement; - encodeResourceToXmlStreamWriter(resource, theEventWriter, false); + encodeResourceToXmlStreamWriter(resource, theEventWriter, false, true); theEventWriter.writeEndElement(); break; } @@ -711,7 +711,7 @@ public class XmlParser extends BaseParser implements IParser { } } - private void encodeResourceToXmlStreamWriter(IBaseResource theResource, XMLStreamWriter theEventWriter, boolean theIncludedResource) throws XMLStreamException, DataFormatException { + private void encodeResourceToXmlStreamWriter(IBaseResource theResource, XMLStreamWriter theEventWriter, boolean theIncludedResource, boolean theSubResource) throws XMLStreamException, DataFormatException { IIdType resourceId = null; if (StringUtils.isNotBlank(theResource.getIdElement().getIdPart())) { @@ -727,7 +727,7 @@ public class XmlParser extends BaseParser implements IParser { if (!theIncludedResource) { if (super.shouldEncodeResourceId(theResource) == false) { resourceId = null; - } else if (getEncodeForceResourceId() != null) { + } else if (theSubResource == false && getEncodeForceResourceId() != null) { resourceId = getEncodeForceResourceId(); } } diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java index 3b8bbc6c497..1238ab9c75f 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/rest/client/GenericClientDstu3Test.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.StringReader; import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.util.*; import org.apache.commons.io.IOUtils; @@ -26,6 +27,7 @@ import org.apache.http.ProtocolVersion; import org.apache.http.client.ClientProtocolException; import org.apache.http.client.HttpClient; import org.apache.http.client.methods.HttpEntityEnclosingRequestBase; +import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.message.BasicHeader; import org.apache.http.message.BasicStatusLine; @@ -735,6 +737,93 @@ public class GenericClientDstu3Test { } } + @Test + public void testPutDoesntForceAllIdsXml() throws Exception { + IParser p = ourCtx.newXmlParser(); + + Patient patient = new Patient(); + patient.setId("PATIENT1"); + patient.addName().addFamily("PATIENT1"); + + Bundle bundle = new Bundle(); + bundle.setId("BUNDLE1"); + bundle.addEntry().setResource(patient); + + final String encoded = p.encodeResourceToString(bundle); + assertEquals("", encoded); + + ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); + when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse); + when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); + when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_XML + "; charset=UTF-8")); + when(myHttpResponse.getEntity().getContent()).thenAnswer(new Answer() { + @Override + public ReaderInputStream answer(InvocationOnMock theInvocation) throws Throwable { + return new ReaderInputStream(new StringReader(encoded), Charset.forName("UTF-8")); + } + }); + + IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir"); + int idx = 0; + + //@formatter:off + client + .update() + .resource(bundle) + .prefer(PreferReturnEnum.REPRESENTATION) + .execute(); + + HttpPut httpRequest = (HttpPut) capt.getValue(); + assertEquals("http://example.com/fhir/Bundle/BUNDLE1", httpRequest.getURI().toASCIIString()); + + String requestString = IOUtils.toString(httpRequest.getEntity().getContent(), StandardCharsets.UTF_8); + assertEquals(encoded, requestString); + } + + @Test + public void testPutDoesntForceAllIdsJson() throws Exception { + IParser p = ourCtx.newJsonParser(); + + Patient patient = new Patient(); + patient.setId("PATIENT1"); + patient.addName().addFamily("PATIENT1"); + + Bundle bundle = new Bundle(); + bundle.setId("BUNDLE1"); + bundle.addEntry().setResource(patient); + + final String encoded = p.encodeResourceToString(bundle); + assertEquals("{\"resourceType\":\"Bundle\",\"id\":\"BUNDLE1\",\"entry\":[{\"resource\":{\"resourceType\":\"Patient\",\"id\":\"PATIENT1\",\"name\":[{\"family\":[\"PATIENT1\"]}]}}]}", encoded); + + ArgumentCaptor capt = ArgumentCaptor.forClass(HttpUriRequest.class); + when(myHttpClient.execute(capt.capture())).thenReturn(myHttpResponse); + when(myHttpResponse.getStatusLine()).thenReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, "OK")); + when(myHttpResponse.getEntity().getContentType()).thenReturn(new BasicHeader("content-type", Constants.CT_FHIR_JSON_NEW + "; charset=UTF-8")); + when(myHttpResponse.getEntity().getContent()).thenAnswer(new Answer() { + @Override + public ReaderInputStream answer(InvocationOnMock theInvocation) throws Throwable { + return new ReaderInputStream(new StringReader(encoded), Charset.forName("UTF-8")); + } + }); + + IGenericClient client = ourCtx.newRestfulGenericClient("http://example.com/fhir"); + int idx = 0; + + //@formatter:off + client + .update() + .resource(bundle) + .prefer(PreferReturnEnum.REPRESENTATION) + .encodedJson() + .execute(); + + HttpPut httpRequest = (HttpPut) capt.getValue(); + assertEquals("http://example.com/fhir/Bundle/BUNDLE1", httpRequest.getURI().toASCIIString()); + + String requestString = IOUtils.toString(httpRequest.getEntity().getContent(), StandardCharsets.UTF_8); + assertEquals(encoded, requestString); + } + @Test public void testResponseHasContentTypeMissing() throws Exception { IParser p = ourCtx.newXmlParser(); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 2bc243911e8..0c2edf322b3 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -49,6 +49,12 @@ part of the generated server conformance statement + + When performing an update using the client on a resource that + contains other resources (e.g. Bundle update), all child resources in the + parent bundle were incorrectly given the ID of the parent. Thanks + to Filip Domazet for reporting! +