From aef7ea5e9fb4f8eb2aeb3d3db82969f49fa6452d Mon Sep 17 00:00:00 2001 From: James Agnew Date: Tue, 5 Aug 2014 14:57:53 -0400 Subject: [PATCH] Correctly encode JSON composite elements with extensions --- hapi-fhir-base/src/changes/changes.xml | 5 + .../context/BaseRuntimeChildDefinition.java | 5 + .../context/BaseRuntimeElementDefinition.java | 5 + .../java/ca/uhn/fhir/parser/JsonParser.java | 108 ++++++++--------- .../ca/uhn/fhir/parser/JsonParserTest.java | 110 +++++++++++++----- 5 files changed, 141 insertions(+), 92 deletions(-) diff --git a/hapi-fhir-base/src/changes/changes.xml b/hapi-fhir-base/src/changes/changes.xml index ae272f85629..299efed6545 100644 --- a/hapi-fhir-base/src/changes/changes.xml +++ b/hapi-fhir-base/src/changes/changes.xml @@ -23,6 +23,11 @@ FHIR Tester UI now correctly sends UTF-8 charset in responses so that message payloads containing non US-ASCII characters will correctly display in the browser + + JSON parser was incorrectly encoding extensions on composite elements outside the element itself + (as is done correctly for non-composite elements) instead of inside of them. Thanks to David Hay of + Orion for reporting this! + diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeChildDefinition.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeChildDefinition.java index ea65eabe96b..bdaf9c6fba5 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeChildDefinition.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeChildDefinition.java @@ -30,6 +30,11 @@ public abstract class BaseRuntimeChildDefinition { public abstract IAccessor getAccessor(); + @Override + public String toString() { + return getClass().getSimpleName()+"[" + getElementName() + "]"; + } + public abstract BaseRuntimeElementDefinition getChildByName(String theName); public abstract BaseRuntimeElementDefinition getChildElementDefinitionByDatatype(Class theType); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeElementDefinition.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeElementDefinition.java index 8ac4ee6ebf4..01f82665ec5 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeElementDefinition.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeElementDefinition.java @@ -56,6 +56,11 @@ public abstract class BaseRuntimeElementDefinition { myImplementingClass = theImplementingClass; } + @Override + public String toString() { + return getClass().getSimpleName()+"[" + getName() + "]"; + } + public void addExtension(RuntimeChildDeclaredExtensionDefinition theExtension) { if (theExtension == null) { throw new NullPointerException(); 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 d8369758a61..7da253813aa 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 @@ -181,7 +181,7 @@ public class JsonParser extends BaseParser implements IParser { for (BundleEntry nextEntry : theBundle.getEntries()) { eventWriter.writeStartObject(); - boolean deleted = nextEntry.getDeletedAt() !=null&&nextEntry.getDeletedAt().isEmpty()==false; + boolean deleted = nextEntry.getDeletedAt() != null && nextEntry.getDeletedAt().isEmpty() == false; if (deleted) { writeTagWithTextNode(eventWriter, "deleted", nextEntry.getDeletedAt()); } @@ -227,11 +227,16 @@ public class JsonParser extends BaseParser implements IParser { eventWriter.flush(); } - private void encodeChildElementToStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, JsonGenerator theWriter, IElement theValue, BaseRuntimeElementDefinition theChildDef, String theChildName) throws IOException { + private void encodeChildElementToStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, JsonGenerator theWriter, IElement theValue, BaseRuntimeElementDefinition theChildDef, + String theChildName) throws IOException { switch (theChildDef.getChildType()) { case PRIMITIVE_DATATYPE: { IPrimitiveDatatype value = (IPrimitiveDatatype) theValue; + if (isBlank(value.getValueAsString())) { + break; + } + if (value instanceof IntegerDt) { if (theChildName != null) { theWriter.write(theChildName, ((IntegerDt) value).getValue()); @@ -333,7 +338,8 @@ public class JsonParser extends BaseParser implements IParser { } - private void encodeCompositeElementChildrenToStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, IElement theElement, JsonGenerator theEventWriter, List theChildren) throws IOException { + private void encodeCompositeElementChildrenToStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, IElement theElement, JsonGenerator theEventWriter, + List theChildren) throws IOException { for (BaseRuntimeChildDefinition nextChild : theChildren) { if (nextChild instanceof RuntimeChildNarrativeDefinition) { INarrativeGenerator gen = myContext.getNarrativeGenerator(); @@ -372,14 +378,16 @@ public class JsonParser extends BaseParser implements IParser { if (childDef == null) { super.throwExceptionForUnknownChildType(nextChild, type); } - + boolean primitive = childDef.getChildType() == ChildTypeEnum.PRIMITIVE_DATATYPE; + if (nextChild instanceof RuntimeChildDeclaredExtensionDefinition) { - RuntimeChildDeclaredExtensionDefinition extDef = (RuntimeChildDeclaredExtensionDefinition) nextChild; - if (extDef.isModifier()) { - addToHeldExtensions(valueIdx, modifierExtensions, extDef, nextValue); - } else { - addToHeldExtensions(valueIdx, extensions, extDef, nextValue); - } + // Don't encode extensions +// RuntimeChildDeclaredExtensionDefinition extDef = (RuntimeChildDeclaredExtensionDefinition) nextChild; +// if (extDef.isModifier()) { +// addToHeldExtensions(valueIdx, modifierExtensions, extDef, nextValue); +// } else { +// addToHeldExtensions(valueIdx, extensions, extDef, nextValue); +// } } else { if (currentChildName == null || !currentChildName.equals(childName)) { @@ -398,7 +406,7 @@ public class JsonParser extends BaseParser implements IParser { encodeChildElementToStreamWriter(theResDef, theResource, theEventWriter, nextValue, childDef, null); } - if (nextValue instanceof ISupportsUndeclaredExtensions) { + if (nextValue instanceof ISupportsUndeclaredExtensions && primitive) { List ext = ((ISupportsUndeclaredExtensions) nextValue).getUndeclaredExtensions(); addToHeldExtensions(valueIdx, ext, extensions); @@ -416,61 +424,41 @@ public class JsonParser extends BaseParser implements IParser { } if (extensions.size() > 0 || modifierExtensions.size() > 0) { - // Ignore extensions if we're encoding a resource, since they - // are handled one level up - if (currentChildName != null) { - theEventWriter.writeStartArray('_' + currentChildName); + theEventWriter.writeStartArray('_' + currentChildName); - for (int i = 0; i < valueIdx; i++) { - boolean haveContent = false; - if (extensions.size() > i && extensions.get(i) != null && extensions.get(i).isEmpty() == false) { - haveContent = true; - theEventWriter.writeStartObject(); - theEventWriter.writeStartArray("extension"); - for (HeldExtension nextExt : extensions.get(i)) { - nextExt.write(theResDef, theResource, theEventWriter); - } - theEventWriter.writeEnd(); - theEventWriter.writeEnd(); - } - - if (!haveContent) { - // theEventWriter.writeEnd(); - theEventWriter.writeNull(); + for (int i = 0; i < valueIdx; i++) { + boolean haveContent = false; + if (extensions.size() > i && extensions.get(i) != null && extensions.get(i).isEmpty() == false) { + haveContent = true; + theEventWriter.writeStartObject(); + theEventWriter.writeStartArray("extension"); + for (HeldExtension nextExt : extensions.get(i)) { + nextExt.write(theResDef, theResource, theEventWriter); } + theEventWriter.writeEnd(); + theEventWriter.writeEnd(); } - // if (extensions.size() > 0) { - // - // theEventWriter.name(extType); - // theEventWriter.beginArray(); - // for (ArrayList next : extensions) { - // if (next == null || next.isEmpty()) { - // theEventWriter.nullValue(); - // } else { - // theEventWriter.beginArray(); - // // next.write(theEventWriter); - // theEventWriter.endArray(); - // } - // } - // for (int i = extensions.size(); i < valueIdx; i++) { - // theEventWriter.nullValue(); - // } - // theEventWriter.endArray(); - // } - - theEventWriter.writeEnd(); + if (!haveContent) { + // theEventWriter.writeEnd(); + theEventWriter.writeNull(); + } } + + theEventWriter.writeEnd(); } } } - private void encodeCompositeElementToStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, IElement theElement, JsonGenerator theEventWriter, BaseRuntimeElementCompositeDefinition resDef) throws IOException, DataFormatException { + private void encodeCompositeElementToStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, IElement theElement, JsonGenerator theEventWriter, + BaseRuntimeElementCompositeDefinition resDef) throws IOException, DataFormatException { + extractAndWriteExtensionsAsDirectChild(theElement, theEventWriter, resDef, theResDef, theResource); encodeCompositeElementChildrenToStreamWriter(theResDef, theResource, theElement, theEventWriter, resDef.getExtensions()); encodeCompositeElementChildrenToStreamWriter(theResDef, theResource, theElement, theEventWriter, resDef.getChildren()); } - private void encodeResourceToJsonStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, JsonGenerator theEventWriter, String theObjectNameOrNull, boolean theIsSubElementWithinResource) throws IOException { + private void encodeResourceToJsonStreamWriter(RuntimeResourceDefinition theResDef, IResource theResource, JsonGenerator theEventWriter, String theObjectNameOrNull, + boolean theIsSubElementWithinResource) throws IOException { if (!theIsSubElementWithinResource) { super.containResourcesForEncoding(theResource); } @@ -491,9 +479,8 @@ public class JsonParser extends BaseParser implements IParser { if (theResource instanceof Binary) { Binary bin = (Binary) theResource; theEventWriter.write("contentType", bin.getContentType()); - theEventWriter.write("content",bin.getContentAsBase64()); + theEventWriter.write("content", bin.getContentAsBase64()); } else { - extractAndWriteExtensionsAsDirectChild(theResource, theEventWriter, resDef, theResDef, theResource); encodeCompositeElementToStreamWriter(theResDef, theResource, theResource, theEventWriter, resDef); } theEventWriter.writeEnd(); @@ -506,7 +493,7 @@ public class JsonParser extends BaseParser implements IParser { JsonGenerator eventWriter = createJsonGenerator(theWriter); RuntimeResourceDefinition resDef = myContext.getResourceDefinition(theResource); - encodeResourceToJsonStreamWriter(resDef, theResource, eventWriter, null,false); + encodeResourceToJsonStreamWriter(resDef, theResource, eventWriter, null, false); eventWriter.flush(); } @@ -541,10 +528,10 @@ public class JsonParser extends BaseParser implements IParser { } /** - * This is useful only for the two cases where extensions are encoded as direct children (e.g. not in some object - * called _name): resource extensions, and extension extensions + * This is useful only for the two cases where extensions are encoded as direct children (e.g. not in some object called _name): resource extensions, and extension extensions */ - private void extractAndWriteExtensionsAsDirectChild(IElement theElement, JsonGenerator theEventWriter, BaseRuntimeElementDefinition theElementDef, RuntimeResourceDefinition theResDef, IResource theResource) throws IOException { + private void extractAndWriteExtensionsAsDirectChild(IElement theElement, JsonGenerator theEventWriter, BaseRuntimeElementDefinition theElementDef, RuntimeResourceDefinition theResDef, + IResource theResource) throws IOException { List extensions = new ArrayList(0); List modifierExtensions = new ArrayList(0); @@ -895,7 +882,8 @@ public class JsonParser extends BaseParser implements IParser { } } - private void writeExtensionsAsDirectChild(IResource theResource, JsonGenerator theEventWriter, RuntimeResourceDefinition resDef, List extensions, List modifierExtensions) throws IOException { + private void writeExtensionsAsDirectChild(IResource theResource, JsonGenerator theEventWriter, RuntimeResourceDefinition resDef, List extensions, + List modifierExtensions) throws IOException { if (extensions.isEmpty() == false) { theEventWriter.writeStartArray("extension"); for (HeldExtension next : extensions) { diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java index 64532dad613..738076980ec 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java @@ -83,6 +83,67 @@ public class JsonParserTest { } + @Test + public void testEncodeExtensionInCompositeElement() { + + Conformance c = new Conformance(); + c.addRest().getSecurity().addUndeclaredExtension(false, "http://foo", new StringDt("AAA")); + + String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(c); + ourLog.info(encoded); + + encoded = ourCtx.newJsonParser().setPrettyPrint(false).encodeResourceToString(c); + ourLog.info(encoded); + assertEquals(encoded, "{\"resourceType\":\"Conformance\",\"rest\":[{\"security\":{\"extension\":[{\"url\":\"http://foo\",\"valueString\":\"AAA\"}]}}]}"); + + } + + @Test + public void testEncodeExtensionInPrimitiveElement() { + + Conformance c = new Conformance(); + c.getAcceptUnknown().addUndeclaredExtension(false, "http://foo", new StringDt("AAA")); + + String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(c); + ourLog.info(encoded); + + encoded = ourCtx.newJsonParser().setPrettyPrint(false).encodeResourceToString(c); + ourLog.info(encoded); + assertEquals(encoded, "{\"resourceType\":\"Conformance\",\"_acceptUnknown\":[{\"extension\":[{\"url\":\"http://foo\",\"valueString\":\"AAA\"}]}]}"); + + // Now with a value + ourLog.info("---------------"); + + c = new Conformance(); + c.getAcceptUnknown().setValue(true); + c.getAcceptUnknown().addUndeclaredExtension(false, "http://foo", new StringDt("AAA")); + + encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(c); + ourLog.info(encoded); + + encoded = ourCtx.newJsonParser().setPrettyPrint(false).encodeResourceToString(c); + ourLog.info(encoded); + assertEquals(encoded, "{\"resourceType\":\"Conformance\",\"acceptUnknown\":true,\"_acceptUnknown\":[{\"extension\":[{\"url\":\"http://foo\",\"valueString\":\"AAA\"}]}]}"); + + } + + + + @Test + public void testEncodeExtensionInResourceElement() { + + Conformance c = new Conformance(); +// c.addRest().getSecurity().addUndeclaredExtension(false, "http://foo", new StringDt("AAA")); + c.addUndeclaredExtension(false, "http://foo", new StringDt("AAA")); + + String encoded = ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(c); + ourLog.info(encoded); + + encoded = ourCtx.newJsonParser().setPrettyPrint(false).encodeResourceToString(c); + ourLog.info(encoded); + assertEquals(encoded, "{\"resourceType\":\"Conformance\",\"extension\":[{\"url\":\"http://foo\",\"valueString\":\"AAA\"}]}"); + + } @Test public void testEncodeBinaryResource() { @@ -387,7 +448,18 @@ public class JsonParserTest { assertEquals("Organization/123", ref.getReference().getValue()); } - + + @Test + public void testEncodeExtensionOnEmptyElement() throws Exception { + + ValueSet valueSet = new ValueSet(); + valueSet.addTelecom().addUndeclaredExtension(false, "http://foo", new StringDt("AAA")); + + String encoded = ourCtx.newJsonParser().encodeResourceToString(valueSet); + assertThat(encoded, containsString("\"telecom\":[{\"extension\":[{\"url\":\"http://foo\",\"valueString\":\"AAA\"}]}")); + + } + @Test @@ -402,11 +474,11 @@ public class JsonParserTest { code.setDisplay("someDisplay"); code.addUndeclaredExtension(false, "urn:alt", new StringDt("alt name")); - String encoded = new FhirContext().newJsonParser().encodeResourceToString(valueSet); + String encoded = ourCtx.newJsonParser().encodeResourceToString(valueSet); ourLog.info(encoded); assertThat(encoded, not(containsString("123456"))); - assertThat(encoded, containsString("\"define\":{\"concept\":[{\"code\":\"someCode\",\"display\":\"someDisplay\"}],\"_concept\":[{\"extension\":[{\"url\":\"urn:alt\",\"valueString\":\"alt name\"}]}]}")); + assertEquals("{\"resourceType\":\"ValueSet\",\"define\":{\"concept\":[{\"extension\":[{\"url\":\"urn:alt\",\"valueString\":\"alt name\"}],\"code\":\"someCode\",\"display\":\"someDisplay\"}]}}", encoded); } @@ -500,32 +572,8 @@ public class JsonParserTest { given.addUndeclaredExtension(ext2); String enc = new FhirContext().newJsonParser().encodeResourceToString(patient); ourLog.info(enc); - //@formatter:off - assertThat(enc, containsString(("{" + - " \"resourceType\":\"Patient\"," + - " \"name\":[" + - " {" + - " \"family\":[" + - " \"Shmoe\"" + - " ]," + - " \"given\":[" + - " \"Joe\"" + - " ]" + - " }" + - " ]," + - " \"_name\":[" + - " {" + - " \"extension\":[" + - " {" + - " \"url\":\"http://examples.com#givenext\"," + - " \"valueString\":\"Hello\"" + - " }" + - " ]" + - " }" + - " ]" + - "}").replaceAll(" +", ""))); - //@formatter:on - + assertEquals("{\"resourceType\":\"Patient\",\"name\":[{\"extension\":[{\"url\":\"http://examples.com#givenext\",\"valueString\":\"Hello\"}],\"family\":[\"Shmoe\"],\"given\":[\"Joe\"]}]}", enc); + IParser newJsonParser = new FhirContext().newJsonParser(); StringReader reader = new StringReader(enc); Patient parsed = newJsonParser.parseResource(Patient.class, reader); @@ -837,9 +885,7 @@ public class JsonParserTest { ExtensionDt undeclaredExtension = undeclaredExtensions.get(0); assertEquals("http://hl7.org/fhir/Profile/iso-21090#qualifier", undeclaredExtension.getUrl().getValue()); - fhirCtx.newJsonParser().setPrettyPrint(true).encodeResourceToWriter(obs, new OutputStreamWriter(System.out)); - - IParser jsonParser = fhirCtx.newJsonParser(); + IParser jsonParser = fhirCtx.newJsonParser().setPrettyPrint(true); String encoded = jsonParser.encodeResourceToString(obs); ourLog.info(encoded);