Improve performance in parser when parsing large bundles

hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java

@@ -230,6 +230,7 @@ public class JsonParser extends BaseParser implements IParser {
 
 			parseChildren(object, state);
 
+			state.endingElement();
 			state.endingElement();
 
 			@SuppressWarnings("unchecked")
@@ -1109,6 +1110,7 @@ public class JsonParser extends BaseParser implements IParser {
 
 		parseBundleChildren(object, state);
 
+		state.endingElement();
 		state.endingElement();
 
 		Bundle retVal = state.getObject();
@@ -1191,8 +1193,6 @@ public class JsonParser extends BaseParser implements IParser {
 	}
 
 	private void parseChildren(JsonObject theObject, ParserState<?> theState) {
-		String elementId = null;
-
 		Set<String> keySet = theObject.keySet();
 
 		int allUnderscoreNames = 0;
@@ -1201,23 +1201,6 @@ public class JsonParser extends BaseParser implements IParser {
 		for (String nextName : keySet) {
 			if ("resourceType".equals(nextName)) {
 				continue;
-			} else if ("id".equals(nextName)) {
-				if (theObject.isNull(nextName)) {
-					continue;
-				}
-				elementId = theObject.getString(nextName);
-				if (myContext.getVersion().getVersion() == FhirVersionEnum.DSTU1) {
-					continue;
-				}
-			} else if ("_id".equals(nextName)) {
-				if (theObject.isNull(nextName)) {
-					continue;
-				}
-				// _id is incorrect, but some early examples in the FHIR spec used it
-				if (theObject.get(nextName).getValueType() == ValueType.STRING) {
-					elementId = theObject.getString(nextName);
-				}
-				continue;
 			} else if ("extension".equals(nextName)) {
 				JsonArray array = grabJsonArray(theObject, nextName, "extension");
 				parseExtension(theState, array, false);
@@ -1245,14 +1228,14 @@ public class JsonParser extends BaseParser implements IParser {
 
 		}
 
-		if (elementId != null) {
+//		if (elementId != null) {
-			IBase object = (IBase) theState.getObject();
+//			IBase object = (IBase) theState.getObject();
-			if (object instanceof IIdentifiableElement) {
+//			if (object instanceof IIdentifiableElement) {
-				((IIdentifiableElement) object).setElementSpecificId(elementId);
+//				((IIdentifiableElement) object).setElementSpecificId(elementId);
-			} else if (object instanceof IBaseResource) {
+//			} else if (object instanceof IBaseResource) {
-				((IBaseResource) object).getIdElement().setValue(elementId);
+//				((IBaseResource) object).getIdElement().setValue(elementId);
-			}
+//			}
-		}
+//		}
 
 		/*
 		 * This happens if an element has an extension but no actual value. I.e.
@@ -1394,6 +1377,7 @@ public class JsonParser extends BaseParser implements IParser {
 
 		parseChildren(object, state);
 
+		state.endingElement();
 		state.endingElement();
 
 		return state.getObject();

hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/ParserState.java

@@ -149,14 +149,8 @@ class ParserState<T> {
 		myState.enteringNewElementExtension(theElem, theUrlAttr, theIsModifier);
 	}
 
-	@SuppressWarnings("unchecked")
 	public T getObject() {
-		IBase retVal = myState.getCurrentElement();
+		return myObject;
-		return (T) retVal;
-	}
-
-	public boolean isComplete() {
-		return myObject != null;
 	}
 
 	public boolean isPreResource() {
@@ -191,10 +185,16 @@ class ParserState<T> {
 		return newChildInstance;
 	}
 
+	@SuppressWarnings("unchecked")
 	private void pop() {
 		myPreviousElement = myState.getCurrentElement();
-		myState = myState.myStack;
+		if (myState.myStack != null) {
-		myState.wereBack();
+			myState = myState.myStack;
+			myState.wereBack();
+		} else {
+			myObject = (T) myState.getCurrentElement();
+			myState = null;
+		}
 	}
 
 	private void push(BaseState theState) {
@@ -250,7 +250,9 @@ class ParserState<T> {
 	 * intended for embedded XHTML content
 	 */
 	public void xmlEvent(XMLEvent theNextEvent) {
-		myState.xmlEvent(theNextEvent);
+		if (myState != null) {
+			myState.xmlEvent(theNextEvent);
+		}
 	}
 
 	static ParserState<Bundle> getPreAtomInstance(IParser theParser, FhirContext theContext, Class<? extends IBaseResource> theResourceType, boolean theJsonMode, IParserErrorHandler theErrorHandler) throws DataFormatException {
@@ -772,7 +774,7 @@ class ParserState<T> {
 
 		@Override
 		public void endingElement() throws DataFormatException {
-			// ignore
+			pop();
 		}
 
 		@Override
@@ -795,7 +797,6 @@ class ParserState<T> {
 		@SuppressWarnings("unchecked")
 		@Override
 		public void wereBack() {
-			myObject = (T) myInstance;
 
 			/*
 			 * Stitch together resource references
@@ -1609,9 +1610,6 @@ class ParserState<T> {
 		@Override
 		public void endingElement() {
 			pop();
-			if (myState == null) {
-				myObject = (T) myInstance;
-			}
 		}
 
 		@Override
@@ -1994,11 +1992,6 @@ class ParserState<T> {
 			super(theResourceType);
 		}
 
-		@Override
-		public void endingElement() throws DataFormatException {
-			// ignore
-		}
-
 		@Override
 		public void enteringNewElement(String theNamespaceUri, String theLocalPart) throws DataFormatException {
 			if (!"feed".equals(theLocalPart)) {
@@ -2018,11 +2011,6 @@ class ParserState<T> {
 			super(theResourceType);
 		}
 
-		@Override
-		public void endingElement() throws DataFormatException {
-			// ignore
-		}
-
 		@Override
 		public void enteringNewElement(String theNamespaceUri, String theLocalPart) throws DataFormatException {
 			if (!"Bundle".equals(theLocalPart)) {
@@ -2064,6 +2052,8 @@ class ParserState<T> {
 
 		@Override
 		public void endingElement() throws DataFormatException {
+//			postProcess();
+			stitchBundleCrossReferences();
 			pop();
 		}
 
@@ -2143,6 +2133,10 @@ class ParserState<T> {
 
 		@Override
 		public void wereBack() {
+			postProcess();
+		}
+
+		private void postProcess() {
 			if (myContext.hasDefaultTypeForProfile()) {
 				IBaseMetaType meta = myInstance.getMeta();
 				Class<? extends IBaseResource> wantedProfileType = null;
@@ -2217,8 +2211,13 @@ class ParserState<T> {
 				}
 			});
 
+			populateTarget();
+		}
+
+		private void stitchBundleCrossReferences() {
 			final boolean bundle = "Bundle".equals(myContext.getResourceDefinition(myInstance).getName());
 			if (bundle) {
+				
 				/*
 				 * Stitch together resource references
 				 */
@@ -2258,8 +2257,6 @@ class ParserState<T> {
 				}
 				
 			}
-
-			populateTarget();
 		}
 
 	}
@@ -2307,9 +2304,6 @@ class ParserState<T> {
 		@Override
 		public void wereBack() {
 			super.wereBack();
-			if (myEntry == null && myMutator == null) {
-				myObject = (T) getCurrentElement();
-			}
 
 			IResource nextResource = (IResource) getCurrentElement();
 			String version = ResourceMetadataKeyEnum.VERSION.get(nextResource);
@@ -2353,9 +2347,6 @@ class ParserState<T> {
 		@Override
 		public void wereBack() {
 			super.wereBack();
-			if (myTarget == null) {
-				myObject = (T) getCurrentElement();
-			}
 
 			if (getCurrentElement() instanceof IDomainResource) {
 				IDomainResource elem = (IDomainResource) getCurrentElement();
@@ -2406,12 +2397,6 @@ class ParserState<T> {
 			return true;
 		}
 
-		@SuppressWarnings("unchecked")
-		@Override
-		public void wereBack() {
-			myObject = (T) myTagList;
-		}
-
 	}
 
 	private class PrimitiveState extends BaseState {

hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/XmlParser.java

@@ -242,6 +242,7 @@ public class XmlParser extends BaseParser implements IParser {
 						parserState.attributeValue(name, elem.getValue());
 						break;
 					}
+					case XMLStreamConstants.END_DOCUMENT:
 					case XMLStreamConstants.END_ELEMENT: {
 						if (!heldComments.isEmpty()) {
 							for (String next : heldComments) {
@@ -250,9 +251,9 @@ public class XmlParser extends BaseParser implements IParser {
 							heldComments.clear();
 						}
 						parserState.endingElement();
-						if (parserState.isComplete()) {
+//						if (parserState.isComplete()) {
-							return parserState.getObject();
+//							return parserState.getObject();
-						}
+//						}
 						break;
 					}
 					case XMLStreamConstants.CHARACTERS: {
@@ -273,7 +274,7 @@ public class XmlParser extends BaseParser implements IParser {
 					throw new DataFormatException("DataFormatException at [" + nextEvent.getLocation().toString() + "]: " + e.getMessage(), e);
 				}
 			}
-			return null;
+			return parserState.getObject();
 		} catch (XMLStreamException e) {
 			throw new DataFormatException(e);
 		}

hapi-fhir-structures-dstu/src/test/java/ca/uhn/fhir/parser/JsonParserTest.java

@@ -681,10 +681,6 @@ public class JsonParserTest {
 
 		ListResource parsed = ourCtx.newJsonParser().parseResource(ListResource.class, enc);
 		assertEquals(Patient.class, parsed.getEntryFirstRep().getItem().getResource().getClass());
-
-		enc = enc.replace("\"id\"", "\"_id\"");
-		parsed = ourCtx.newJsonParser().parseResource(ListResource.class, enc);
-		assertEquals(Patient.class, parsed.getEntryFirstRep().getItem().getResource().getClass());
 	}
 
 	@Test

hapi-fhir-structures-hl7org-dstu2/src/test/java/ca/uhn/fhir/parser/JsonParserHl7OrgDstu2Test.java

@@ -645,11 +645,7 @@ public class JsonParserHl7OrgDstu2Test {
 		
 		List_ parsed = ourCtx.newJsonParser().parseResource(List_.class,enc);
 		assertEquals(Patient.class, parsed.getEntry().get(0).getItem().getResource().getClass());
-
+	}
-		enc = enc.replace("\"id\"", "\"_id\"");
-		parsed = ourCtx.newJsonParser().parseResource(List_.class,enc);
-		assertEquals(Patient.class, parsed.getEntry().get(0).getItem().getResource().getClass());
-}
 
 	@Test
 	public void testEncodeInvalidChildGoodException() {

src/changes/changes.xml

@@ -22,9 +22,23 @@
 			</action>
 			<action type="add" issue="346">
 				Server now respects the parameter <![CDATA[<code>_format=application/xml+fhir"</code>]]>
-				which is technically invalid since the + should be escaped, but is likely to be used.
+				which is technically invalid since the + should be escaped, but is likely to be used. Also,
+				a parameter of <![CDATA[<code>_format=html</code>]]> can now be used, which 
+				forces SyntaxHighlightingInterceptor to use HTML even
+				if the headers wouldn't otherwise trigger it.
 				Thanks to Jim Steel for reporting!
 			</action>
+			<action type="fix">
+				Improve performance when parsing large bundles by fixing a loop over all of the
+				entries inthe bundle to stitch together cross-references, which was happening once
+				per entry instead of once overall. Thanks to Erick on the HAPI FHIR Google Group for
+				noticing that this was an issue!
+			</action>
+			<action type="remove">
+				JSON parser no longer allows the resource ID to be specified in an element called "_id"
+				(the correct one is "id"). Previously _id was allowed because some early FHIR examples
+				used that form, but this was never actually valid so it is now being removed.
+			</action>
 		</release>
 		<release version="1.5" date="2016-04-20">
 			<action type="fix" issue="339">