From 6ffb1c8c56226a057ab2efdf947325b454fb24cb Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 25 Mar 2016 19:27:18 +0100 Subject: [PATCH] Fix #312 - Don't fail if extension list contains a null --- .../ca/uhn/fhir/model/api/BaseElement.java | 6 + .../java/ca/uhn/fhir/util/FhirTerser.java | 9 + .../uhn/fhir/parser/XmlParserDstu2Test.java | 19 ++ .../uhn/fhir/parser/XmlParserDstu3Test.java | 175 ++++++++++-------- src/changes/changes.xml | 5 + 5 files changed, 132 insertions(+), 82 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/BaseElement.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/BaseElement.java index f15adb5c6f0..736215ee926 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/BaseElement.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/model/api/BaseElement.java @@ -142,6 +142,9 @@ public abstract class BaseElement implements IElement, ISupportsUndeclaredExtens protected boolean isBaseEmpty() { if (myUndeclaredExtensions != null) { for (ExtensionDt next : myUndeclaredExtensions) { + if (next == null) { + continue; + } if (!next.isEmpty()) { return false; } @@ -149,6 +152,9 @@ public abstract class BaseElement implements IElement, ISupportsUndeclaredExtens } if (myUndeclaredModifierExtensions != null) { for (ExtensionDt next : myUndeclaredModifierExtensions) { + if (next == null) { + continue; + } if (!next.isEmpty()) { return false; } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java index 22a42ce6fbc..de82b9c2c47 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/FhirTerser.java @@ -78,6 +78,9 @@ public class FhirTerser { if (theElement instanceof ISupportsUndeclaredExtensions) { ISupportsUndeclaredExtensions containingElement = (ISupportsUndeclaredExtensions) theElement; for (ExtensionDt nextExt : containingElement.getUndeclaredExtensions()) { + if (nextExt == null) { + continue; + } theCallback.acceptUndeclaredExtension(containingElement, null, theChildDefinition, theDefinition, nextExt); addUndeclaredExtensions(nextExt, theDefinition, theChildDefinition, theCallback); } @@ -85,6 +88,9 @@ public class FhirTerser { if (theElement instanceof IBaseHasExtensions) { for (IBaseExtension nextExt : ((IBaseHasExtensions)theElement).getExtension()) { + if (nextExt == null) { + continue; + } theCallback.acceptElement(nextExt.getValue(), null, theChildDefinition, theDefinition); addUndeclaredExtensions(nextExt, theDefinition, theChildDefinition, theCallback); } @@ -92,6 +98,9 @@ public class FhirTerser { if (theElement instanceof IBaseHasModifierExtensions) { for (IBaseExtension nextExt : ((IBaseHasModifierExtensions)theElement).getModifierExtension()) { + if (nextExt == null) { + continue; + } theCallback.acceptElement(nextExt.getValue(), null, theChildDefinition, theDefinition); addUndeclaredExtensions(nextExt, theDefinition, theChildDefinition, theCallback); } 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 a40bc734daa..48137b44a99 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 @@ -100,6 +100,25 @@ public class XmlParserDstu2Test { private static final FhirContext ourCtx = FhirContext.forDstu2(); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(XmlParserDstu2Test.class); + /** + * See #312 + */ + @Test + public void testEncodeNullExtension() { + Patient patient = new Patient(); + patient.getUndeclaredExtensions().add(null); // Purposely add null + patient.getUndeclaredModifierExtensions().add(null); // Purposely add null + patient.getUndeclaredExtensions().add(new ExtensionDt(false, "http://hello.world", new StringDt("Hello World"))); + patient.getName().add(null); + patient.addName().getFamily().add(null); + + IParser parser = ourCtx.newXmlParser(); + String xml = parser.encodeResourceToString(patient); + + ourLog.info(xml); + assertEquals("", xml); + } + @Test public void testBundleWithBinary() { //@formatter:off diff --git a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java index 00111383256..e090aa62ad5 100644 --- a/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java +++ b/hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/XmlParserDstu3Test.java @@ -102,9 +102,6 @@ public class XmlParserDstu3Test { ourCtx.setNarrativeGenerator(null); } - - - @Test public void testBundleWithBinary() { //@formatter:off @@ -159,7 +156,6 @@ public class XmlParserDstu3Test { assertEquals("ORG", o.getName()); } - @Test public void testDuration() { Encounter enc = new Encounter(); @@ -173,8 +169,7 @@ public class XmlParserDstu3Test { assertThat(str, not(containsString("meta"))); assertThat(str, containsString("")); } - - + @Test public void testEncodeAndParseContained() { IParser xmlParser = ourCtx.newXmlParser().setPrettyPrint(true); @@ -243,7 +238,7 @@ public class XmlParserDstu3Test { assertThat(encoded, not(stringContainsInOrder(Arrays.asList("", "")))); } - + @Test public void testEncodeAndParseExtensionOnCode() { Organization o = new Organization(); @@ -262,7 +257,7 @@ public class XmlParserDstu3Test { assertEquals("acode", code.getValue()); } - + @Test public void testEncodeAndParseExtensionOnReference() { DataElement de = new DataElement(); @@ -287,7 +282,7 @@ public class XmlParserDstu3Test { assertEquals("ORG", o.getName()); } - + @Test public void testEncodeAndParseExtensions() throws Exception { @@ -405,7 +400,6 @@ public class XmlParserDstu3Test { assertEquals("MR", patient.getIdentifier().get(0).getType().getCoding().get(0).getCode()); } - @Test public void testEncodeAndParseMetaProfileAndTags() { Patient p = new Patient(); @@ -473,8 +467,7 @@ public class XmlParserDstu3Test { assertEquals("sec_term2", tagList.get(1).getCode()); assertEquals("sec_label2", tagList.get(1).getDisplay()); } - - + @Test public void testEncodeAndParseMetaProfiles() { Patient p = new Patient(); @@ -844,8 +837,6 @@ public class XmlParserDstu3Test { assertThat(encoded, not(containsString("tag"))); } - - /** * #158 */ @@ -864,6 +855,8 @@ public class XmlParserDstu3Test { assertThat(encoded, not(containsString("Label"))); } + + @Test public void testEncodeExtensionWithResourceContent() { IParser parser = ourCtx.newXmlParser(); @@ -945,6 +938,25 @@ public class XmlParserDstu3Test { } + /** + * See #312 + */ + @Test + public void testEncodeNullExtension() { + Patient patient = new Patient(); + patient.getExtension().add(null); // Purposely add null + patient.getModifierExtension().add(null); // Purposely add null + patient.getExtension().add(new Extension("http://hello.world", new StringType("Hello World"))); + patient.getName().add(null); + patient.addName().getFamily().add(null); + + IParser parser = ourCtx.newXmlParser(); + String xml = parser.encodeResourceToString(patient); + + ourLog.info(xml); + assertEquals("", xml); + } + @Test public void testEncodeReferenceUsingUnqualifiedResourceWorksCorrectly() { @@ -1022,55 +1034,6 @@ public class XmlParserDstu3Test { assertThat(encoded, not(containsString("maritalStatus"))); } - @Test - public void testEncodeWithEncodeElements() throws Exception { - Patient patient = new Patient(); - patient.getMeta().addProfile("http://profile"); - patient.addName().addFamily("FAMILY"); - patient.addAddress().addLine("LINE1"); - - Bundle bundle = new Bundle(); - bundle.setTotal(100); - bundle.addEntry().setResource(patient); - - { - IParser p = ourCtx.newXmlParser(); - p.setEncodeElements(new HashSet(Arrays.asList("Patient.name", "Bundle.entry"))); - p.setPrettyPrint(true); - String out = p.encodeResourceToString(bundle); - ourLog.info(out); - assertThat(out, not(containsString("total"))); - assertThat(out, (containsString("Patient"))); - assertThat(out, (containsString("name"))); - assertThat(out, not(containsString("address"))); - } - { - IParser p = ourCtx.newXmlParser(); - p.setEncodeElements(new HashSet(Arrays.asList("Patient.name"))); - p.setEncodeElementsAppliesToResourceTypes(new HashSet(Arrays.asList("Patient"))); - p.setPrettyPrint(true); - String out = p.encodeResourceToString(bundle); - ourLog.info(out); - assertThat(out, (containsString("total"))); - assertThat(out, (containsString("Patient"))); - assertThat(out, (containsString("name"))); - assertThat(out, not(containsString("address"))); - } - { - IParser p = ourCtx.newXmlParser(); - p.setEncodeElements(new HashSet(Arrays.asList("Patient"))); - p.setEncodeElementsAppliesToResourceTypes(new HashSet(Arrays.asList("Patient"))); - p.setPrettyPrint(true); - String out = p.encodeResourceToString(bundle); - ourLog.info(out); - assertThat(out, (containsString("total"))); - assertThat(out, (containsString("Patient"))); - assertThat(out, (containsString("name"))); - assertThat(out, (containsString("address"))); - } - - } - @Test public void testEncodeWithDontEncodeElements() throws Exception { Patient patient = new Patient(); @@ -1138,6 +1101,55 @@ public class XmlParserDstu3Test { assertThat(out, not(containsString("meta"))); } } + + @Test + public void testEncodeWithEncodeElements() throws Exception { + Patient patient = new Patient(); + patient.getMeta().addProfile("http://profile"); + patient.addName().addFamily("FAMILY"); + patient.addAddress().addLine("LINE1"); + + Bundle bundle = new Bundle(); + bundle.setTotal(100); + bundle.addEntry().setResource(patient); + + { + IParser p = ourCtx.newXmlParser(); + p.setEncodeElements(new HashSet(Arrays.asList("Patient.name", "Bundle.entry"))); + p.setPrettyPrint(true); + String out = p.encodeResourceToString(bundle); + ourLog.info(out); + assertThat(out, not(containsString("total"))); + assertThat(out, (containsString("Patient"))); + assertThat(out, (containsString("name"))); + assertThat(out, not(containsString("address"))); + } + { + IParser p = ourCtx.newXmlParser(); + p.setEncodeElements(new HashSet(Arrays.asList("Patient.name"))); + p.setEncodeElementsAppliesToResourceTypes(new HashSet(Arrays.asList("Patient"))); + p.setPrettyPrint(true); + String out = p.encodeResourceToString(bundle); + ourLog.info(out); + assertThat(out, (containsString("total"))); + assertThat(out, (containsString("Patient"))); + assertThat(out, (containsString("name"))); + assertThat(out, not(containsString("address"))); + } + { + IParser p = ourCtx.newXmlParser(); + p.setEncodeElements(new HashSet(Arrays.asList("Patient"))); + p.setEncodeElementsAppliesToResourceTypes(new HashSet(Arrays.asList("Patient"))); + p.setPrettyPrint(true); + String out = p.encodeResourceToString(bundle); + ourLog.info(out); + assertThat(out, (containsString("total"))); + assertThat(out, (containsString("Patient"))); + assertThat(out, (containsString("name"))); + assertThat(out, (containsString("address"))); + } + + } @Test public void testEncodeWithNarrative() { @@ -1711,8 +1723,7 @@ public class XmlParserDstu3Test { assertEquals(Reference.class, ext.getValue().getClass()); assertEquals("#2179414-cm", ((Reference) ext.getValue()).getReference()); assertEquals(ConceptMap.class, ((Reference) ext.getValue()).getResource().getClass()); - - + ourLog.info(ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(de)); assertThat(output, containsString("http://hl7.org/fhir/StructureDefinition/11179-permitted-value-valueset")); @@ -1721,7 +1732,7 @@ public class XmlParserDstu3Test { ourLog.info("Actual : {}", output); assertEquals(input, output); } - + @Test public void testParseAndEncodeNestedExtensions() { //@formatter:off @@ -1735,24 +1746,24 @@ public class XmlParserDstu3Test { " \n" + ""; //@formatter:on - + Patient p = ourCtx.newXmlParser().parseResource(Patient.class, input); - DateType bd = p.getBirthDateElement(); + DateType bd = p.getBirthDateElement(); assertEquals("2005-03-04", bd.getValueAsString()); - + List exts = bd.getExtensionsByUrl("http://my.fancy.extension.url"); assertEquals(1, exts.size()); Extension ext = exts.get(0); assertEquals(null, ext.getValue()); - + exts = ext.getExtensionsByUrl("http://my.fancy.extension.url"); assertEquals(1, exts.size()); ext = exts.get(0); - assertEquals("myNestedValue", ((StringType)ext.getValue()).getValue()); - + assertEquals("myNestedValue", ((StringType) ext.getValue()).getValue()); + String encoded = ourCtx.newXmlParser().setPrettyPrint(true).encodeResourceToString(p); ourLog.info(encoded); - + //@formatter:off assertThat(encoded, stringContainsInOrder( "", @@ -1765,7 +1776,7 @@ public class XmlParserDstu3Test { "", "")); //@formatter:on - + } @Test @@ -1788,8 +1799,7 @@ public class XmlParserDstu3Test { assertEquals("urn:oid:0.1.2.3", parsed.getEntry().get(0).getResource().getIdElement().getValue()); } - - + @Test public void testParseBundleNewWithPlaceholderIdsInBase1() { //@formatter:off @@ -1945,7 +1955,8 @@ public class XmlParserDstu3Test { /** * see #144 and #146 */ - @Test @Ignore + @Test + @Ignore public void testParseContained() { FhirContext c = FhirContext.forDstu3(); @@ -2006,9 +2017,9 @@ public class XmlParserDstu3Test { assertEquals(1, actual.getContent().size()); /* - * If this fails, it's possibe the DocumentManifest structure is wrong: - * It should be - * @Child(name = "p", type = {Attachment.class, ValueSet.class}, order=1, min=1, max=1, modifier=false, summary=true) + * If this fails, it's possibe the DocumentManifest structure is wrong: It should be + * + * @Child(name = "p", type = {Attachment.class, ValueSet.class}, order=1, min=1, max=1, modifier=false, summary=true) */ assertNotNull(((Reference) actual.getContent().get(0).getP()).getResource()); } @@ -2134,7 +2145,7 @@ public class XmlParserDstu3Test { } - //TODO: this should work + // TODO: this should work @Test @Ignore public void testParseNarrative() throws Exception { @@ -2167,7 +2178,7 @@ public class XmlParserDstu3Test { " \n" + ""; //@formatter:on - + try { ourCtx.newXmlParser().parseResource(Patient.class, input); fail(); diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 1ae7dafbc7a..5b57905ce44 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -327,6 +327,11 @@
]]> Thanks to Peter Van Houte from Agfa for the amazing work! + + Parser failed with a NPE while encoding resources if the + resource contained a null extension. Thanks to + steve1medix for reporting! +