Parser: Reporting unexpected repeating element for choice (#4574) (#4590)

* Parser: Reporting unexpected repeating element for choice (#4574)

IParserErrorHandler.unexpectedRepeatingElement is now also called for
unexpected repeating elements of type choice, where concrete names
may be different based on the choice's data type.

* setting test-utilities java version to the test version

* added changelog

* added Max to the developer liss

* removed unneeded dependency

---------

Co-authored-by: patrick-werner <pa.f.werner@gmail.com>

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

@@ -25,6 +25,7 @@ import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition;
 import ca.uhn.fhir.context.BaseRuntimeElementDefinition;
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.FhirVersionEnum;
+import ca.uhn.fhir.context.RuntimeChildChoiceDefinition;
 import ca.uhn.fhir.context.RuntimeChildDeclaredExtensionDefinition;
 import ca.uhn.fhir.context.RuntimePrimitiveDatatypeDefinition;
 import ca.uhn.fhir.context.RuntimePrimitiveDatatypeNarrativeDefinition;
@@ -583,10 +584,19 @@ class ParserState<T> {
 				return;
 			}
 
-			if ((child.getMax() == 0 || child.getMax() == 1) && !myParsedNonRepeatableNames.add(theChildName)) {
+			if ((child.getMax() == 0 || child.getMax() == 1)) {
-				myErrorHandler.unexpectedRepeatingElement(null, theChildName);
+				String nameToCheck;
-				push(new SwallowChildrenWholeState(getPreResourceState()));
+				if (child instanceof RuntimeChildChoiceDefinition) {
-				return;
+					RuntimeChildChoiceDefinition choiceChild = (RuntimeChildChoiceDefinition) child;
+					nameToCheck = choiceChild.getField().getName();
+				} else {
+					nameToCheck = theChildName;
+				}
+				if(!myParsedNonRepeatableNames.add(nameToCheck)) {
+					myErrorHandler.unexpectedRepeatingElement(null, nameToCheck);
+					push(new SwallowChildrenWholeState(getPreResourceState()));
+					return;
+				}
 			}
 
 			BaseRuntimeElementDefinition<?> target = child.getChildByName(theChildName);

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_8_0/4574-fix-repeating-choice-data-type-to-throw-error.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 4468
+title: "When parsing a FHIR resource which has multiple value[x] choice fields,  the parser does not call the IParserErrorHandler#unexpectedRepeatingElement method on the configured parser error handler. Thanks to Max Bureck for the 
+  pull request!"

hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/Dstu2_1JsonParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class Dstu2_1JsonParserErrorHandlerTest extends AbstractJsonParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forDstu2_1();
+
+	@Override
+	protected FhirContext getFhirContext() {
+		return ourCtx;
+	}
+}

hapi-fhir-structures-dstu2.1/src/test/java/ca/uhn/fhir/parser/Dstu2_1XmlParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class Dstu2_1XmlParserErrorHandlerTest extends AbstractXmlParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forDstu2_1();
+
+	@Override
+	protected FhirContext getFhirContext() {
+		return ourCtx;
+	}
+}

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

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class Dstu2JsonParserErrorHandlerTest extends AbstractJsonParserErrorHandlerTest {
+
+	private static FhirContext ourCtx = FhirContext.forDstu2();
+
+	@Override
+	protected FhirContext getFhirContext() {
+		return ourCtx;
+	}
+}

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

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class Dstu2XmlParserErrorHandlerTest extends AbstractXmlParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forDstu2();
+
+	@Override
+	protected FhirContext getFhirContext() {
+		return ourCtx;
+	}
+}

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/Dstu3JsonParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class Dstu3JsonParserErrorHandlerTest extends AbstractJsonParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forDstu3();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

hapi-fhir-structures-dstu3/src/test/java/ca/uhn/fhir/parser/Dstu3XmlParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class Dstu3XmlParserErrorHandlerTest extends AbstractXmlParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forDstu3();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

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

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class Dstu2Hl7OrgJsonParserErrorHandlerTest extends AbstractJsonParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forDstu2Hl7Org();
+
+	@Override
+	protected FhirContext getFhirContext() {
+		return ourCtx;
+	}
+}

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

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class Dstu2Hl7OrgXmlParserErrorHandlerTest extends AbstractXmlParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forDstu2Hl7Org();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/R4JsonParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class R4JsonParserErrorHandlerTest extends AbstractJsonParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forR4();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/parser/R4XmlParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class R4XmlParserErrorHandlerTest extends AbstractXmlParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forR4();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

hapi-fhir-structures-r4b/src/test/java/ca/uhn/fhir/parser/R4BJsonParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class R4BJsonParserErrorHandlerTest extends AbstractJsonParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forR4B();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

hapi-fhir-structures-r4b/src/test/java/ca/uhn/fhir/parser/R4BXmlParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class R4BXmlParserErrorHandlerTest extends AbstractXmlParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forR4B();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

hapi-fhir-structures-r5/src/test/java/ca/uhn/fhir/parser/R5JsonParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class R5JsonParserErrorHandlerTest extends AbstractJsonParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forR5();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

hapi-fhir-structures-r5/src/test/java/ca/uhn/fhir/parser/R5XmlParserErrorHandlerTest.java

@@ -0,0 +1,13 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public class R5XmlParserErrorHandlerTest extends AbstractXmlParserErrorHandlerTest {
+
+  private static FhirContext ourCtx = FhirContext.forR5();
+
+  @Override
+  protected FhirContext getFhirContext() {
+    return ourCtx;
+  }
+}

hapi-fhir-test-utilities/pom.xml

@@ -15,6 +15,13 @@
 	<name>HAPI FHIR Test Utilities</name>
 	<url>http://jamesagnew.github.io/hapi-fhir/</url>
 
+	<properties>
+		<!-- this is a test jar, so use our test settings -->
+		<maven.compiler.source>${maven.compiler.testSource}</maven.compiler.source>
+		<maven.compiler.target>${maven.compiler.testTarget}</maven.compiler.target>
+		<maven.compiler.release>${maven.compiler.testRelease}</maven.compiler.release>
+	</properties>
+
 	<dependencies>
 
 		<dependency>

hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/parser/AbstractJsonParserErrorHandlerTest.java

@@ -0,0 +1,28 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+abstract public non-sealed class AbstractJsonParserErrorHandlerTest extends AbstractParserErrorHandlerTest {
+
+	private static String PATIENT_DUPLICATE_CHOICE = 
+			""" 
+			{
+				"resourceType": "Patient",
+				"deceasedBoolean": "true",
+				"deceasedDateTime": "2022-02-07T13:28:17-05:00"
+			}
+			""";
+
+	protected abstract FhirContext getFhirContext();
+	
+	@Override
+	protected IParser createParser() {
+		return getFhirContext().newJsonParser();
+	}
+
+	@Override
+	protected String createResourceWithRepeatingChoice() {
+		return PATIENT_DUPLICATE_CHOICE;
+	}
+
+}

hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/parser/AbstractParserErrorHandlerTest.java

@@ -0,0 +1,41 @@
+package ca.uhn.fhir.parser;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import org.junit.jupiter.api.Test;
+/**
+ * Defines FHIR version independent tests for testing parser error handling. In version dependent
+ * projects, the sub-types {@link AbstractXmlParserErrorHandlerTest}, {@link
+ * AbstractJsonParserErrorHandlerTest} can be sub-classed to create a complete test.
+ */
+public abstract sealed class AbstractParserErrorHandlerTest
+    permits AbstractXmlParserErrorHandlerTest, AbstractJsonParserErrorHandlerTest {
+
+  protected abstract IParser createParser();
+
+  protected abstract String createResourceWithRepeatingChoice();
+
+	@Test
+	public void testRepeatingChoiceHandled() {
+
+		// Let error handler throw custom exception on unexpectedRepeatingElement
+		@SuppressWarnings("serial")
+		class RepeatingChoiceHandledException extends RuntimeException {}
+		IParserErrorHandler errorHandler = new ErrorHandlerAdapter() {
+			@Override
+			public void unexpectedRepeatingElement(IParseLocation theLocation, String theElementName) {
+			  throw new RepeatingChoiceHandledException();
+			}
+		};
+
+		IParser parser = createParser();
+		parser.setParserErrorHandler(errorHandler);
+
+		String resourceStr = createResourceWithRepeatingChoice();
+		assertThrows(
+			RepeatingChoiceHandledException.class,
+			() -> {
+				parser.parseResource(resourceStr);
+		});
+	}
+}

hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/parser/AbstractXmlParserErrorHandlerTest.java

@@ -0,0 +1,26 @@
+package ca.uhn.fhir.parser;
+
+import ca.uhn.fhir.context.FhirContext;
+
+public abstract non-sealed class AbstractXmlParserErrorHandlerTest extends AbstractParserErrorHandlerTest {
+
+	private static String PATIENT_DUPLICATE_CHOICE = 
+			"""
+			<Patient xmlns="http://hl7.org/fhir">
+					<deceasedBoolean value="true"></deceasedBoolean>
+					<deceasedDateTime value="2022-02-07T13:28:17-05:00"></deceasedDateTime>
+			</Patient>""";
+
+	protected abstract FhirContext getFhirContext();
+
+	@Override
+	protected IParser createParser() {
+		return getFhirContext().newXmlParser();
+	}
+
+	@Override
+	protected String createResourceWithRepeatingChoice() {
+		return PATIENT_DUPLICATE_CHOICE;
+	}
+
+}

pom.xml

@@ -878,6 +878,11 @@
 			<name>Dominique Villard</name>
 			<organization>Doctolib</organization>
 		</developer>
+		<developer>
+			<id>boereck</id>
+			<name>Max Bureck</name>
+			<organization>Fraunhofer FOKUS</organization>
+		</developer>
 	</developers>
 
 	<licenses>