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 d5de2bb198f..b5eb3ec6137 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 @@ -22,16 +22,25 @@ package ca.uhn.fhir.parser; import ca.uhn.fhir.context.*; import ca.uhn.fhir.context.BaseRuntimeChildDefinition.IMutator; -import ca.uhn.fhir.model.api.*; +import ca.uhn.fhir.model.api.ExtensionDt; +import ca.uhn.fhir.model.api.IElement; +import ca.uhn.fhir.model.api.IIdentifiableElement; +import ca.uhn.fhir.model.api.IResource; +import ca.uhn.fhir.model.api.ISupportsUndeclaredExtensions; +import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; +import ca.uhn.fhir.model.api.Tag; +import ca.uhn.fhir.model.api.TagList; import ca.uhn.fhir.model.api.annotation.Child; -import ca.uhn.fhir.model.base.composite.BaseResourceReferenceDt; import ca.uhn.fhir.model.base.resource.ResourceMetadataMap; import ca.uhn.fhir.model.primitive.IdDt; import ca.uhn.fhir.model.primitive.InstantDt; import ca.uhn.fhir.model.primitive.XhtmlDt; import ca.uhn.fhir.parser.json.JsonLikeValue.ScalarType; import ca.uhn.fhir.parser.json.JsonLikeValue.ValueType; -import ca.uhn.fhir.util.*; +import ca.uhn.fhir.util.BundleUtil; +import ca.uhn.fhir.util.FhirTerser; +import ca.uhn.fhir.util.ReflectionUtil; +import ca.uhn.fhir.util.XmlUtil; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.tuple.Pair; @@ -39,9 +48,17 @@ import org.hl7.fhir.instance.model.api.*; import javax.xml.stream.events.StartElement; import javax.xml.stream.events.XMLEvent; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; -import static org.apache.commons.lang3.StringUtils.*; +import static org.apache.commons.lang3.StringUtils.defaultIfBlank; +import static org.apache.commons.lang3.StringUtils.isNotBlank; +import static org.apache.commons.lang3.StringUtils.isNotEmpty; class ParserState { @@ -54,6 +71,8 @@ class ParserState { private T myObject; private IBase myPreviousElement; private BaseState myState; + private List myGlobalResources = new ArrayList<>(); + private List myGlobalReferences = new ArrayList<>(); private ParserState(IParser theParser, FhirContext theContext, boolean theJsonMode, IParserErrorHandler theErrorHandler) { myParser = theParser; @@ -130,7 +149,6 @@ class ParserState { } } - public void string(String theData) { myState.string(theData); } @@ -145,6 +163,60 @@ class ParserState { } } + public IBase newInstance(RuntimeChildDeclaredExtensionDefinition theDefinition) { + return theDefinition.newInstance(); + } + + public ICompositeType newCompositeInstance(PreResourceState thePreResourceState, BaseRuntimeChildDefinition theChild, BaseRuntimeElementCompositeDefinition theCompositeTarget) { + ICompositeType retVal = (ICompositeType) theCompositeTarget.newInstance(theChild.getInstanceConstructorArguments()); + if (retVal instanceof IBaseReference) { + IBaseReference ref = (IBaseReference) retVal; + myGlobalReferences.add(ref); + thePreResourceState.getLocalReferences().add(ref); + } + return retVal; + } + + public ICompositeType newCompositeTypeInstance(PreResourceState thePreResourceState, BaseRuntimeElementCompositeDefinition theCompositeTarget) { + ICompositeType retVal = (ICompositeType) theCompositeTarget.newInstance(); + if (retVal instanceof IBaseReference) { + IBaseReference ref = (IBaseReference) retVal; + myGlobalReferences.add(ref); + thePreResourceState.getLocalReferences().add(ref); + } + return retVal; + } + + public IPrimitiveType newPrimitiveInstance(RuntimeChildDeclaredExtensionDefinition theDefinition, RuntimePrimitiveDatatypeDefinition thePrimitiveTarget) { + return thePrimitiveTarget.newInstance(theDefinition.getInstanceConstructorArguments()); + } + + public IPrimitiveType getPrimitiveInstance(BaseRuntimeChildDefinition theChild, RuntimePrimitiveDatatypeDefinition thePrimitiveTarget) { + return thePrimitiveTarget.newInstance(theChild.getInstanceConstructorArguments()); + } + + public IBaseXhtml newInstance(RuntimePrimitiveDatatypeXhtmlHl7OrgDefinition theXhtmlTarget) { + return theXhtmlTarget.newInstance(); + } + + public XhtmlDt newInstance(RuntimePrimitiveDatatypeNarrativeDefinition theXhtmlTarget) { + return theXhtmlTarget.newInstance(); + } + + public IPrimitiveType newInstance(RuntimePrimitiveDatatypeDefinition thePrimitiveTarget) { + return thePrimitiveTarget.newInstance(); + } + + public IBaseResource newInstance(RuntimeResourceDefinition theDef) { + IBaseResource retVal = theDef.newInstance(); + myGlobalResources.add(retVal); + return retVal; + } + + public IBase newInstance(RuntimeResourceBlockDefinition theBlockTarget) { + return theBlockTarget.newInstance(); + } + private abstract class BaseState { private PreResourceState myPreResourceState; @@ -256,7 +328,7 @@ class ParserState { private class ContainedResourcesStateHapi extends PreResourceState { public ContainedResourcesStateHapi(PreResourceState thePreResourcesState) { - super(thePreResourcesState, ((IResource) thePreResourcesState.myInstance).getStructureFhirVersionEnum()); + super(thePreResourcesState, thePreResourcesState.myInstance.getStructureFhirVersionEnum()); } @Override @@ -290,7 +362,6 @@ class ParserState { @SuppressWarnings("unchecked") List containedResources = (List) preResCurrentElement.getContained().getContainedResources(); containedResources.add(res); - } } @@ -374,7 +445,7 @@ class ParserState { switch (target.getChildType()) { case COMPOSITE_DATATYPE: { BaseRuntimeElementCompositeDefinition compositeTarget = (BaseRuntimeElementCompositeDefinition) target; - ICompositeType newChildInstance = (ICompositeType) compositeTarget.newInstance(myDefinition.getInstanceConstructorArguments()); + ICompositeType newChildInstance = newCompositeInstance(getPreResourceState(), myDefinition, compositeTarget); myDefinition.getMutator().addValue(myParentInstance, newChildInstance); ElementCompositeState newState = new ElementCompositeState(myPreResourceState, theLocalPart, compositeTarget, newChildInstance); push(newState); @@ -383,7 +454,7 @@ class ParserState { case ID_DATATYPE: case PRIMITIVE_DATATYPE: { RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target; - IPrimitiveType newChildInstance = primitiveTarget.newInstance(myDefinition.getInstanceConstructorArguments()); + IPrimitiveType newChildInstance = newPrimitiveInstance(myDefinition, primitiveTarget); myDefinition.getMutator().addValue(myParentInstance, newChildInstance); PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance); push(newState); @@ -404,7 +475,7 @@ class ParserState { RuntimeChildDeclaredExtensionDefinition declaredExtension = myDefinition.getChildExtensionForUrl(theUrlAttr); if (declaredExtension != null) { if (myChildInstance == null) { - myChildInstance = myDefinition.newInstance(); + myChildInstance = newInstance(myDefinition); myDefinition.getMutator().addValue(myParentInstance, myChildInstance); } BaseState newState = new DeclaredExtensionState(getPreResourceState(), declaredExtension, myChildInstance); @@ -414,6 +485,7 @@ class ParserState { } } + @Override protected IBase getCurrentElement() { return myParentInstance; @@ -501,7 +573,7 @@ class ParserState { switch (target.getChildType()) { case COMPOSITE_DATATYPE: { BaseRuntimeElementCompositeDefinition compositeTarget = (BaseRuntimeElementCompositeDefinition) target; - ICompositeType newChildInstance = (ICompositeType) compositeTarget.newInstance(child.getInstanceConstructorArguments()); + ICompositeType newChildInstance = newCompositeInstance(getPreResourceState(), child, compositeTarget); child.getMutator().addValue(myInstance, newChildInstance); ParserState.ElementCompositeState newState = new ElementCompositeState(getPreResourceState(), theChildName, compositeTarget, newChildInstance); push(newState); @@ -511,7 +583,7 @@ class ParserState { case PRIMITIVE_DATATYPE: { RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target; IPrimitiveType newChildInstance; - newChildInstance = primitiveTarget.newInstance(child.getInstanceConstructorArguments()); + newChildInstance = getPrimitiveInstance(child, primitiveTarget); child.getMutator().addValue(myInstance, newChildInstance); PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance); push(newState); @@ -519,7 +591,7 @@ class ParserState { } case RESOURCE_BLOCK: { RuntimeResourceBlockDefinition blockTarget = (RuntimeResourceBlockDefinition) target; - IBase newBlockInstance = blockTarget.newInstance(); + IBase newBlockInstance = newInstance(blockTarget); child.getMutator().addValue(myInstance, newBlockInstance); ElementCompositeState newState = new ElementCompositeState(getPreResourceState(), theChildName, blockTarget, newBlockInstance); push(newState); @@ -527,7 +599,7 @@ class ParserState { } case PRIMITIVE_XHTML: { RuntimePrimitiveDatatypeNarrativeDefinition xhtmlTarget = (RuntimePrimitiveDatatypeNarrativeDefinition) target; - XhtmlDt newDt = xhtmlTarget.newInstance(); + XhtmlDt newDt = newInstance(xhtmlTarget); child.getMutator().addValue(myInstance, newDt); XhtmlState state = new XhtmlState(getPreResourceState(), newDt, true); push(state); @@ -535,7 +607,7 @@ class ParserState { } case PRIMITIVE_XHTML_HL7ORG: { RuntimePrimitiveDatatypeXhtmlHl7OrgDefinition xhtmlTarget = (RuntimePrimitiveDatatypeXhtmlHl7OrgDefinition) target; - IBaseXhtml newDt = xhtmlTarget.newInstance(); + IBaseXhtml newDt = newInstance(xhtmlTarget); child.getMutator().addValue(myInstance, newDt); XhtmlStateHl7Org state = new XhtmlStateHl7Org(getPreResourceState(), newDt); push(state); @@ -669,7 +741,7 @@ class ParserState { switch (target.getChildType()) { case COMPOSITE_DATATYPE: { BaseRuntimeElementCompositeDefinition compositeTarget = (BaseRuntimeElementCompositeDefinition) target; - ICompositeType newChildInstance = (ICompositeType) compositeTarget.newInstance(); + ICompositeType newChildInstance = newCompositeTypeInstance(getPreResourceState(), compositeTarget); myExtension.setValue(newChildInstance); ElementCompositeState newState = new ElementCompositeState(getPreResourceState(), theLocalPart, compositeTarget, newChildInstance); push(newState); @@ -678,7 +750,7 @@ class ParserState { case ID_DATATYPE: case PRIMITIVE_DATATYPE: { RuntimePrimitiveDatatypeDefinition primitiveTarget = (RuntimePrimitiveDatatypeDefinition) target; - IPrimitiveType newChildInstance = primitiveTarget.newInstance(); + IPrimitiveType newChildInstance = newInstance(primitiveTarget); myExtension.setValue(newChildInstance); PrimitiveState newState = new PrimitiveState(getPreResourceState(), newChildInstance); push(newState); @@ -847,10 +919,10 @@ class ParserState { private abstract class PreResourceState extends BaseState { private Map myContainedResources; + private List myLocalReferences = new ArrayList<>(); private IBaseResource myInstance; private FhirVersionEnum myParentVersion; private Class myResourceType; - PreResourceState(Class theResourceType) { super(null); myResourceType = theResourceType; @@ -869,6 +941,10 @@ class ParserState { myContainedResources = thePreResourcesState.getContainedResources(); } + public List getLocalReferences() { + return myLocalReferences; + } + @Override public void endingElement() throws DataFormatException { stitchBundleCrossReferences(); @@ -905,10 +981,10 @@ class ParserState { if (!definition.getName().equals(theLocalPart) && definition.getName().equalsIgnoreCase(theLocalPart)) { throw new DataFormatException("Unknown resource type '" + theLocalPart + "': Resource names are case sensitive, found similar name: '" + definition.getName() + "'"); } - myInstance = def.newInstance(); + myInstance = newInstance(def); if (myInstance instanceof IResource) { - push(new ResourceStateHapi(getRootPreResourceState(), def, (IResource) myInstance)); + push(new ResourceStateHapi(getRootPreResourceState(), def, (IResource) myInstance, myContainedResources)); } else { push(new ResourceStateHl7Org(getRootPreResourceState(), def, myInstance)); } @@ -996,8 +1072,7 @@ class ParserState { /* * Stitch together resource references */ - List resources = t.getAllPopulatedChildElementsOfType(myInstance, IBaseResource.class); - for (IBaseResource next : resources) { + for (IBaseResource next : myGlobalResources) { IIdType id = next.getIdElement(); if (id != null && id.isEmpty() == false) { String resName = myContext.getResourceDefinition(next).getName(); @@ -1006,15 +1081,12 @@ class ParserState { } } - for (IBaseResource next : resources) { - List refs = myContext.newTerser().getAllPopulatedChildElementsOfType(next, IBaseReference.class); - for (IBaseReference nextRef : refs) { - if (nextRef.isEmpty() == false && nextRef.getReferenceElement() != null) { - IIdType unqualifiedVersionless = nextRef.getReferenceElement().toUnqualifiedVersionless(); - IBaseResource target = idToResource.get(unqualifiedVersionless.getValueAsString()); - if (target != null) { - nextRef.setResource(target); - } + for (IBaseReference nextRef : myGlobalReferences) { + if (nextRef.isEmpty() == false && nextRef.getReferenceElement() != null) { + IIdType unqualifiedVersionless = nextRef.getReferenceElement().toUnqualifiedVersionless(); + IBaseResource target = idToResource.get(unqualifiedVersionless.getValueAsString()); + if (target != null) { + nextRef.setResource(target); } } } @@ -1035,43 +1107,21 @@ class ParserState { } void weaveContainedResources() { - FhirTerser terser = myContext.newTerser(); - terser.visit(myInstance, new IModelVisitor() { - - @Override - public void acceptElement(IBaseResource theResource, IBase theElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, - BaseRuntimeElementDefinition theDefinition) { - if (theElement instanceof BaseResourceReferenceDt) { - BaseResourceReferenceDt nextRef = (BaseResourceReferenceDt) theElement; - String ref = nextRef.getReference().getValue(); - if (isNotBlank(ref)) { - if (ref.startsWith("#")) { - IResource target = (IResource) myContainedResources.get(ref); - if (target != null) { - ourLog.debug("Resource contains local ref {} in field {}", ref, thePathToElement); - nextRef.setResource(target); - } else { - myErrorHandler.unknownReference(null, ref); - } - } - } - } else if (theElement instanceof IBaseReference) { - IBaseReference nextRef = (IBaseReference) theElement; - String ref = nextRef.getReferenceElement().getValue(); - if (isNotBlank(ref)) { - if (ref.startsWith("#")) { - IBaseResource target = myContainedResources.get(ref); - if (target != null) { - ourLog.debug("Resource contains local ref {} in field {}", ref, thePathToElement); - nextRef.setResource(target); - } else { - myErrorHandler.unknownReference(null, ref); - } - } + for (IBaseReference nextRef : myLocalReferences) { + String ref = nextRef.getReferenceElement().getValue(); + if (isNotBlank(ref)) { + if (ref.startsWith("#")) { + IBaseResource target = myContainedResources.get(ref); + if (target != null) { + ourLog.debug("Resource contains local ref {}", ref); + nextRef.setResource(target); + } else { + myErrorHandler.unknownReference(null, ref); } } } - }); + } + } @Override @@ -1284,7 +1334,7 @@ class ParserState { private IResource myInstance; - public ResourceStateHapi(PreResourceState thePreResourceState, BaseRuntimeElementCompositeDefinition theDef, IResource theInstance) { + public ResourceStateHapi(PreResourceState thePreResourceState, BaseRuntimeElementCompositeDefinition theDef, IResource theInstance, Map theContainedResources) { super(thePreResourceState, theDef.getName(), theDef, theInstance); myInstance = theInstance; } 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 d47934000da..2ed7221cc16 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 @@ -181,9 +181,6 @@ public class XmlParser extends BaseParser { heldComments.clear(); } parserState.endingElement(); -// if (parserState.isComplete()) { -// return parserState.getObject(); -// } break; } case XMLStreamConstants.CHARACTERS: { @@ -707,12 +704,12 @@ public class XmlParser extends BaseParser { theEventWriter.writeStartElement(prefix, se.getName().getLocalPart(), namespaceURI); theEventWriter.writeNamespace(prefix, namespaceURI); } - for (Iterator iter= se.getAttributes(); iter.hasNext(); ) { - Attribute next = iter.next(); - if ("lang".equals(next.getName().getLocalPart())) { - theEventWriter.writeAttribute("", "", next.getName().getLocalPart(), next.getValue()); - } - } +// for (Iterator iter= se.getAttributes(); iter.hasNext(); ) { +// Attribute next = iter.next(); +// if ("lang".equals(next.getName().getLocalPart())) { +// theEventWriter.writeAttribute("", "", next.getName().getLocalPart(), next.getValue()); +// } +// } firstElement = false; } else { if (isBlank(se.getName().getPrefix())) { @@ -729,10 +726,10 @@ public class XmlParser extends BaseParser { } else { theEventWriter.writeStartElement(se.getName().getPrefix(), se.getName().getLocalPart(), se.getName().getNamespaceURI()); } - for (Iterator attrIter = se.getAttributes(); attrIter.hasNext(); ) { - Attribute next = (Attribute) attrIter.next(); - theEventWriter.writeAttribute(next.getName().getLocalPart(), next.getValue()); - } + } + for (Iterator attrIter = se.getAttributes(); attrIter.hasNext(); ) { + Attribute next = (Attribute) attrIter.next(); + theEventWriter.writeAttribute(next.getName().getLocalPart(), next.getValue()); } break; case XMLStreamConstants.DTD: 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 72fcefa9ca3..786b92afd43 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 @@ -160,7 +160,7 @@ public class FhirTerser { public List getAllPopulatedChildElementsOfType(IBaseResource theResource, final Class theType) { final ArrayList retVal = new ArrayList<>(); BaseRuntimeElementCompositeDefinition def = myContext.getResourceDefinition(theResource); - visit(new IdentityHashMap<>(), theResource, theResource, null, null, def, new IModelVisitor() { + visit(newMap(), theResource, theResource, null, null, def, new IModelVisitor() { @SuppressWarnings("unchecked") @Override public void acceptElement(IBaseResource theOuterResource, IBase theElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, BaseRuntimeElementDefinition theDefinition) { @@ -179,7 +179,7 @@ public class FhirTerser { public List getAllResourceReferences(final IBaseResource theResource) { final ArrayList retVal = new ArrayList<>(); BaseRuntimeElementCompositeDefinition def = myContext.getResourceDefinition(theResource); - visit(new IdentityHashMap<>(), theResource, theResource, null, null, def, new IModelVisitor() { + visit(newMap(), theResource, theResource, null, null, def, new IModelVisitor() { @Override public void acceptElement(IBaseResource theOuterResource, IBase theElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, BaseRuntimeElementDefinition theDefinition) { if (theElement == null || theElement.isEmpty()) { @@ -793,7 +793,11 @@ public class FhirTerser { */ public void visit(IBaseResource theResource, IModelVisitor theVisitor) { BaseRuntimeElementCompositeDefinition def = myContext.getResourceDefinition(theResource); - visit(new IdentityHashMap<>(), theResource, theResource, null, null, def, theVisitor); + visit(newMap(), theResource, theResource, null, null, def, theVisitor); + } + + public Map newMap() { + return new IdentityHashMap<>(); } /** @@ -814,7 +818,7 @@ public class FhirTerser { visit(theResource, null, def, theVisitor, new ArrayList<>(), new ArrayList<>(), new ArrayList<>()); } - private void visit(IdentityHashMap theStack, IBaseResource theResource, IBase theElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, + private void visit(Map theStack, IBaseResource theResource, IBase theElement, List thePathToElement, BaseRuntimeChildDefinition theChildDefinition, BaseRuntimeElementDefinition theDefinition, IModelVisitor theCallback) { List pathToElement = addNameToList(thePathToElement, theChildDefinition); diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml index 79336532b05..bbf874540b6 100644 --- a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/4_2_0/changes.yaml @@ -5,6 +5,7 @@ (dependent HAPI modules listed in brackets):
  • Jetty (CLI): 9.4.14.v20181114 -> 9.4.23.v20191118
    • +
  • Spring (JPA, Testpage): 5.2.1 -> 5.2.3 (addresses CVE-2020-5398 and CVE-2020-5397)
" - item: issue: "1583" @@ -64,6 +65,12 @@ issue: "1649" type: "add" title: "Support for LOINC 2.67 file format changes has been added to the JPA Server LOINC uploader. Thanks to Dan Vreeman for reporting!" +- item: + issue: "1660" + type: "perf" + title: "A significant performance improvement was added to the Json and XML parsers when parsing large Bundle + resources. Throughput for parsing these resources has been improved by roughly 50%. Thanks to Rok Bertoncelj + and Bogdan Solga for providing analysis and insight that triggered this change." - item: issue: "1658" type: "fix" diff --git a/pom.xml b/pom.xml index 16d54eeadcd..0364f1a9b00 100644 --- a/pom.xml +++ b/pom.xml @@ -655,7 +655,7 @@ 9.5.1-5_1 1.2_5 1.7.28 - 5.2.1.RELEASE + 5.2.3.RELEASE 2.2.0.RELEASE 2.2.0.RELEASE