From acdbdc0be7dedb69ac7496cdda196509987c73d7 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 25 Aug 2016 07:32:37 -0400 Subject: [PATCH] Fix #426 - Extension with datatype of ID failed to parse --- .../java/ca/uhn/fhir/parser/ParserState.java | 103 +++++++++--------- .../jpa/dao/dstu3/FhirSystemDaoDstu3Test.java | 15 +++ .../uhn/fhir/parser/XmlParserDstu3Test.java | 64 ++++++++++- src/changes/changes.xml | 4 + 4 files changed, 129 insertions(+), 57 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java index 63aee5d8acf..233b9cf8709 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java @@ -10,7 +10,7 @@ package ca.uhn.fhir.parser; * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -105,6 +105,7 @@ class ParserState { private final IParser myParser; private IBase myPreviousElement; private BaseState myState; + private ParserState(IParser theParser, FhirContext theContext, boolean theJsonMode, IParserErrorHandler theErrorHandler) { myParser = theParser; myContext = theContext; @@ -223,7 +224,8 @@ class ParserState { } } - static ParserState getPreAtomInstance(IParser theParser, FhirContext theContext, Class theResourceType, boolean theJsonMode, IParserErrorHandler theErrorHandler) throws DataFormatException { + static ParserState getPreAtomInstance(IParser theParser, FhirContext theContext, Class theResourceType, boolean theJsonMode, IParserErrorHandler theErrorHandler) + throws DataFormatException { ParserState retVal = new ParserState(theParser, theContext, theJsonMode, theErrorHandler); if (theContext.getVersion().getVersion() == FhirVersionEnum.DSTU1) { retVal.push(retVal.new PreAtomState(theResourceType)); @@ -237,7 +239,8 @@ class ParserState { * @param theResourceType * May be null */ - static ParserState getPreResourceInstance(IParser theParser, Class theResourceType, FhirContext theContext, boolean theJsonMode, IParserErrorHandler theErrorHandler) throws DataFormatException { + static ParserState getPreResourceInstance(IParser theParser, Class theResourceType, FhirContext theContext, boolean theJsonMode, IParserErrorHandler theErrorHandler) + throws DataFormatException { ParserState retVal = new ParserState(theParser, theContext, theJsonMode, theErrorHandler); if (theResourceType == null) { if (theContext.getVersion().getVersion().isRi()) { @@ -1389,7 +1392,7 @@ class ParserState { @Override public void wereBack() { super.wereBack(); - + IResource res = (IResource) getCurrentElement(); assert res != null; if (res.getId() == null || res.getId().isEmpty()) { @@ -1580,7 +1583,7 @@ class ParserState { return; } } - + /* * This means we've found an element that doesn't exist on the structure. If the error handler doesn't throw * an exception, swallow the element silently along with any child elements @@ -1740,10 +1743,10 @@ class ParserState { } if ("id".equals(theName)) { if (getCurrentElement() instanceof IBaseElement) { - ((IBaseElement)getCurrentElement()).setId(theValue); + ((IBaseElement) getCurrentElement()).setId(theValue); return; } else if (getCurrentElement() instanceof IIdentifiableElement) { - ((IIdentifiableElement)getCurrentElement()).setElementSpecificId(theValue); + ((IIdentifiableElement) getCurrentElement()).setElementSpecificId(theValue); return; } } @@ -1771,42 +1774,35 @@ class ParserState { } BaseRuntimeElementDefinition target = myContext.getRuntimeChildUndeclaredExtensionDefinition().getChildByName(theLocalPart); - if (target == null) { - myErrorHandler.unknownElement(null, theLocalPart); - push(new SwallowChildrenWholeState(getPreResourceState())); - return; + + if (target != null) { + switch (target.getChildType()) { + case COMPOSITE_DATATYPE: { + BaseRuntimeElementCompositeDefinition compositeTarget = (BaseRuntimeElementCompositeDefinition) target; + ICompositeType newChildInstance = (ICompositeType) compositeTarget.newInstance(); + myExtension.setValue(newChildInstance); + ElementCompositeState newState = new ElementCompositeState(getPreResourceState(), compositeTarget, newChildInstance); + push(newState); + return; + } + case ID_DATATYPE: + case PRIMITIVE_DATATYPE: { + RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target; + IPrimitiveType newChildInstance = primitiveTarget.newInstance(); + myExtension.setValue(newChildInstance); + PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance); + push(newState); + return; + } + } } - switch (target.getChildType()) { - case COMPOSITE_DATATYPE: { - BaseRuntimeElementCompositeDefinition compositeTarget = (BaseRuntimeElementCompositeDefinition) target; - ICompositeType newChildInstance = (ICompositeType) compositeTarget.newInstance(); - myExtension.setValue(newChildInstance); - ElementCompositeState newState = new ElementCompositeState(getPreResourceState(), compositeTarget, newChildInstance); - push(newState); - return; - } - case PRIMITIVE_DATATYPE: { - RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target; - IPrimitiveType newChildInstance = primitiveTarget.newInstance(); - myExtension.setValue(newChildInstance); - PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance); - push(newState); - return; - } - case PRIMITIVE_XHTML: - case RESOURCE: - case RESOURCE_BLOCK: - case UNDECL_EXT: - case EXTENSION_DECLARED: - case CONTAINED_RESOURCES: - case CONTAINED_RESOURCE_LIST: - case ID_DATATYPE: - case PRIMITIVE_XHTML_HL7ORG: - break; - } + // We hit an invalid type for the extension + myErrorHandler.unknownElement(null, theLocalPart); + push(new SwallowChildrenWholeState(getPreResourceState())); + return; } - + @Override protected IBaseExtension getCurrentElement() { return myExtension; @@ -1989,7 +1985,7 @@ class ParserState { @Override public void endingElement() throws DataFormatException { -// postProcess(); + // postProcess(); stitchBundleCrossReferences(); pop(); } @@ -2090,11 +2086,11 @@ class ParserState { } } } - + if (wantedProfileType != null && !wantedProfileType.equals(myInstance.getClass())) { if (myResourceType == null || myResourceType.isAssignableFrom(wantedProfileType)) { - ourLog.debug("Converting resource of type {} to type defined for profile \"{}\": {}", new Object[] {myInstance.getClass().getName(), usedProfile, wantedProfileType}); - + ourLog.debug("Converting resource of type {} to type defined for profile \"{}\": {}", new Object[] { myInstance.getClass().getName(), usedProfile, wantedProfileType }); + /* * This isn't the most efficient thing really.. If we want a specific * type we just re-parse into that type. The problem is that we don't know @@ -2110,7 +2106,7 @@ class ParserState { } } } - + FhirTerser terser = myContext.newTerser(); terser.visit(myInstance, new IModelVisitor() { @@ -2146,7 +2142,8 @@ class ParserState { } @Override - public void acceptUndeclaredExtension(ISupportsUndeclaredExtensions theContainingElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, BaseRuntimeElementDefinition theDefinition, ExtensionDt theNextExt) { + public void acceptUndeclaredExtension(ISupportsUndeclaredExtensions theContainingElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, + BaseRuntimeElementDefinition theDefinition, ExtensionDt theNextExt) { acceptElement(theNextExt.getValue(), null, null, null); } }); @@ -2157,7 +2154,7 @@ class ParserState { private void stitchBundleCrossReferences() { final boolean bundle = "Bundle".equals(myContext.getResourceDefinition(myInstance).getName()); if (bundle) { - + /* * Stitch together resource references */ @@ -2195,7 +2192,7 @@ class ParserState { } } } - + } } @@ -2224,11 +2221,11 @@ class ParserState { assert theResourceType == null || IResource.class.isAssignableFrom(theResourceType); } -// @Override -// public void enteringNewElement(String theNamespaceUri, String theLocalPart) throws DataFormatException { -// super.enteringNewElement(theNamespaceUri, theLocalPart); -// populateTarget(); -// } + // @Override + // public void enteringNewElement(String theNamespaceUri, String theLocalPart) throws DataFormatException { + // super.enteringNewElement(theNamespaceUri, theLocalPart); + // populateTarget(); + // } @Override protected void populateTarget() { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java index 6dda5e19ac0..32bb17d2676 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3Test.java @@ -802,6 +802,21 @@ public class FhirSystemDaoDstu3Test extends BaseJpaDstu3SystemTest { myPatientDao.read(new IdType("Patient/" + methodName), mySrd); } + @Test + public void testTransactionCreateWithPutUsingAbsoluteUrl() { + String methodName = "testTransactionCreateWithPutUsingAbsoluteUrl"; + Bundle request = new Bundle(); + request.setType(BundleType.TRANSACTION); + + Patient p = new Patient(); + p.addIdentifier().setSystem("urn:system").setValue(methodName); + request.addEntry().setResource(p).getRequest().setMethod(HTTPVerb.PUT).setUrl("http://localhost/server/base/Patient/" + methodName); + + mySystemDao.transaction(mySrd, request); + + myPatientDao.read(new IdType("Patient/" + methodName), mySrd); + } + @Test public void testTransactionCreateWithPutUsingUrl2() throws Exception { String req = IOUtils.toString(FhirSystemDaoDstu3Test.class.getResourceAsStream("/bundle-dstu3.xml"), StandardCharsets.UTF_8); 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 82c86d5f081..77adf7b90e4 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 @@ -135,7 +135,7 @@ public class XmlParserDstu3Test { o = (Organization) rr.getResource(); assertEquals("ORG", o.getName()); } - + @Test public void testDuration() { Encounter enc = new Encounter(); @@ -186,7 +186,7 @@ public class XmlParserDstu3Test { assertEquals("Organization/orgid", pt.getManagingOrganization().getReferenceElement().getValue()); assertSame(org, pt.getManagingOrganization().getResource()); } - + @Test public void testEncodeAndParseCompositeExtension() { PatientWithCustomCompositeExtension pat = new PatientWithCustomCompositeExtension(); @@ -203,8 +203,7 @@ public class XmlParserDstu3Test { assertEquals("ValueA", pat.getFooParentExtension().getChildA().getValue()); assertEquals("ValueB", pat.getFooParentExtension().getChildB().getValue()); } - - + @Test public void testEncodeAndParseContained() { IParser xmlParser = ourCtx.newXmlParser().setPrettyPrint(true); @@ -329,6 +328,7 @@ public class XmlParserDstu3Test { ourCtx = null; } + @Test public void testEncodeAndParseContainedNonCustomTypes() { ourCtx = FhirContext.forDstu3(); @@ -2692,6 +2692,62 @@ public class XmlParserDstu3Test { assertNotNull(((Reference) actual.getContent().get(0).getP()).getResource()); } + /** + * See #426 + */ + @Test + public void testParseExtensionWithIdType() { + //@formatter:off + String input = + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + //@formatter:on + Patient pt = ourCtx.newXmlParser().parseResource(Patient.class, input); + + List extList = pt.getExtensionsByUrl("http://aaa.ch/fhir/Patient#mangedcare"); + extList = extList.get(0).getExtensionsByUrl("http://aaa.ch/fhir/Patient#mangedcare-aaa-id"); + Extension ext = extList.get(0); + IdType value = (IdType) ext.getValue(); + assertEquals("mc1", value.getValueAsString()); + } + + /** + * See #426 + * + * Value type of FOO isn't a valid datatype + */ + @Test + public void testParseExtensionWithInvalidType() { + //@formatter:off + String input = + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + //@formatter:on + Patient pt = ourCtx.newXmlParser().parseResource(Patient.class, input); + + List extList = pt.getExtensionsByUrl("http://aaa.ch/fhir/Patient#mangedcare"); + extList = extList.get(0).getExtensionsByUrl("http://aaa.ch/fhir/Patient#mangedcare-aaa-id"); + Extension ext = extList.get(0); + IdType value = (IdType) ext.getValue(); + assertEquals(null, value); + } + /** * See #342 */ diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 62cdeffb1b5..5aa90e34a4a 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -197,6 +197,10 @@ Fix NullPointerException when encoding an extension containing CodeableConcept with log level set to TRACE. Thanks to Bill Denton for the report! + + Parser failed to parse resources containing an extension with a value type of + "id". Thanks to Raphael Mäder for reporting! +