Fix FhirTerser to handle Enumeration correctly (#5726)

* Add test for CDA with binary database blog storage and some logging for the Terser.

* More logging and a new FhirTerser test.

* Add more comments and prove Enumeration is a valid subtype of Type.

* More logging and comments.

* James solution:  Comment out some code.  Spotless.

* Get rid of logging and TODOs.  Use AtomicBoolean for test.

* Try just commenting out the continue; line.

* Integrate changes from ja_20240222_choice_specialization_fix.

* Spotless.

* Changelog and fix for bad import.

* Code review feedback.

hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeChildAny.java

@@ -30,7 +30,6 @@ import org.hl7.fhir.instance.model.api.IBaseReference;
 
 import java.lang.reflect.Field;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
@@ -46,13 +45,15 @@ public class RuntimeChildAny extends RuntimeChildChoiceDefinition {
 	void sealAndInitialize(
 			FhirContext theContext,
 			Map<Class<? extends IBase>, BaseRuntimeElementDefinition<?>> theClassToElementDefinitions) {
-		List<Class<? extends IBase>> choiceTypes = new ArrayList<Class<? extends IBase>>();
+		List<Class<? extends IBase>> choiceTypes = new ArrayList<>();
+		List<Class<? extends IBase>> specializationChoiceTypes = new ArrayList<>();
 
 		for (Class<? extends IBase> next : theClassToElementDefinitions.keySet()) {
 			if (next.equals(XhtmlDt.class)) {
 				continue;
 			}
 
+			boolean isSpecialization = false;
 			BaseRuntimeElementDefinition<?> nextDef = theClassToElementDefinitions.get(next);
 			if (nextDef instanceof IRuntimeDatatypeDefinition) {
 				if (((IRuntimeDatatypeDefinition) nextDef).isSpecialization()) {
@@ -60,7 +61,7 @@ public class RuntimeChildAny extends RuntimeChildChoiceDefinition {
 					 * Things like BoundCodeDt shoudn't be considered as valid options for an "any" choice, since
 					 * we'll already have CodeDt as an option
 					 */
-					continue;
+					isSpecialization = true;
 				}
 			}
 
@@ -68,28 +69,36 @@ public class RuntimeChildAny extends RuntimeChildChoiceDefinition {
 					|| IDatatype.class.isAssignableFrom(next)
 					|| IBaseDatatype.class.isAssignableFrom(next)
 					|| IBaseReference.class.isAssignableFrom(next)) {
-				choiceTypes.add(next);
+				if (isSpecialization) {
-			}
+					specializationChoiceTypes.add(next);
-		}
-		Collections.sort(choiceTypes, new Comparator<Class<?>>() {
-			@Override
-			public int compare(Class<?> theO1, Class<?> theO2) {
-				boolean o1res = IResource.class.isAssignableFrom(theO1);
-				boolean o2res = IResource.class.isAssignableFrom(theO2);
-				if (o1res && o2res) {
-					return theO1.getSimpleName().compareTo(theO2.getSimpleName());
-				} else if (o1res) {
-					return -1;
-				} else if (o1res == false && o2res == false) {
-					return 0;
 				} else {
-					return 1;
+					choiceTypes.add(next);
 				}
 			}
-		});
+		}
 
-		setChoiceTypes(choiceTypes);
+		choiceTypes.sort(new ResourceTypeNameComparator());
+		specializationChoiceTypes.sort(new ResourceTypeNameComparator());
+
+		setChoiceTypes(choiceTypes, specializationChoiceTypes);
 
 		super.sealAndInitialize(theContext, theClassToElementDefinitions);
 	}
+
+	private static class ResourceTypeNameComparator implements Comparator<Class<?>> {
+		@Override
+		public int compare(Class<?> theO1, Class<?> theO2) {
+			boolean o1res = IResource.class.isAssignableFrom(theO1);
+			boolean o2res = IResource.class.isAssignableFrom(theO2);
+			if (o1res && o2res) {
+				return theO1.getSimpleName().compareTo(theO2.getSimpleName());
+			} else if (o1res) {
+				return -1;
+			} else if (!o2res) {
+				return 0;
+			} else {
+				return 1;
+			}
+		}
+	}
 }

hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeChildChoiceDefinition.java

@@ -22,7 +22,9 @@ package ca.uhn.fhir.context;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.model.api.annotation.Child;
 import ca.uhn.fhir.model.api.annotation.Description;
+import jakarta.annotation.Nonnull;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.Validate;
 import org.hl7.fhir.instance.model.api.IBase;
 import org.hl7.fhir.instance.model.api.IBaseDatatype;
 import org.hl7.fhir.instance.model.api.IBaseReference;
@@ -45,6 +47,7 @@ public class RuntimeChildChoiceDefinition extends BaseRuntimeDeclaredChildDefini
 	private Map<Class<? extends IBase>, BaseRuntimeElementDefinition<?>> myDatatypeToElementDefinition;
 	private String myReferenceSuffix;
 	private List<Class<? extends IBaseResource>> myResourceTypes;
+	private List<Class<? extends IBase>> mySpecializationChoiceTypes = Collections.emptyList();
 
 	/**
 	 * Constructor
@@ -70,8 +73,13 @@ public class RuntimeChildChoiceDefinition extends BaseRuntimeDeclaredChildDefini
 		super(theField, theChildAnnotation, theDescriptionAnnotation, theElementName);
 	}
 
-	void setChoiceTypes(List<Class<? extends IBase>> theChoiceTypes) {
+	void setChoiceTypes(
+			@Nonnull List<Class<? extends IBase>> theChoiceTypes,
+			@Nonnull List<Class<? extends IBase>> theSpecializationChoiceTypes) {
+		Validate.notNull(theChoiceTypes, "theChoiceTypes must not be null");
+		Validate.notNull(theSpecializationChoiceTypes, "theSpecializationChoiceTypes must not be null");
 		myChoiceTypes = Collections.unmodifiableList(theChoiceTypes);
+		mySpecializationChoiceTypes = Collections.unmodifiableList(theSpecializationChoiceTypes);
 	}
 
 	public List<Class<? extends IBase>> getChoices() {
@@ -96,14 +104,28 @@ public class RuntimeChildChoiceDefinition extends BaseRuntimeDeclaredChildDefini
 	void sealAndInitialize(
 			FhirContext theContext,
 			Map<Class<? extends IBase>, BaseRuntimeElementDefinition<?>> theClassToElementDefinitions) {
-		myNameToChildDefinition = new HashMap<String, BaseRuntimeElementDefinition<?>>();
+		myNameToChildDefinition = new HashMap<>();
-		myDatatypeToElementName = new HashMap<Class<? extends IBase>, String>();
+		myDatatypeToElementName = new HashMap<>();
-		myDatatypeToElementDefinition = new HashMap<Class<? extends IBase>, BaseRuntimeElementDefinition<?>>();
+		myDatatypeToElementDefinition = new HashMap<>();
-		myResourceTypes = new ArrayList<Class<? extends IBaseResource>>();
+		myResourceTypes = new ArrayList<>();
 
 		myReferenceSuffix = "Reference";
 
-		for (Class<? extends IBase> next : myChoiceTypes) {
+		sealAndInitializeChoiceTypes(theContext, theClassToElementDefinitions, mySpecializationChoiceTypes, true);
+		sealAndInitializeChoiceTypes(theContext, theClassToElementDefinitions, myChoiceTypes, false);
+
+		myNameToChildDefinition = Collections.unmodifiableMap(myNameToChildDefinition);
+		myDatatypeToElementName = Collections.unmodifiableMap(myDatatypeToElementName);
+		myDatatypeToElementDefinition = Collections.unmodifiableMap(myDatatypeToElementDefinition);
+		myResourceTypes = Collections.unmodifiableList(myResourceTypes);
+	}
+
+	private void sealAndInitializeChoiceTypes(
+			FhirContext theContext,
+			Map<Class<? extends IBase>, BaseRuntimeElementDefinition<?>> theClassToElementDefinitions,
+			List<Class<? extends IBase>> choiceTypes,
+			boolean theIsSpecilization) {
+		for (Class<? extends IBase> next : choiceTypes) {
 
 			String elementName = null;
 			BaseRuntimeElementDefinition<?> nextDef;
@@ -112,8 +134,10 @@ public class RuntimeChildChoiceDefinition extends BaseRuntimeDeclaredChildDefini
 				elementName = getElementName() + StringUtils.capitalize(next.getSimpleName());
 				nextDef = findResourceReferenceDefinition(theClassToElementDefinitions);
 
-				myNameToChildDefinition.put(getElementName() + "Reference", nextDef);
+				if (!theIsSpecilization) {
-				myNameToChildDefinition.put(getElementName() + "Resource", nextDef);
+					myNameToChildDefinition.put(getElementName() + "Reference", nextDef);
+					myNameToChildDefinition.put(getElementName() + "Resource", nextDef);
+				}
 
 				myResourceTypes.add((Class<? extends IBaseResource>) next);
 
@@ -147,21 +171,23 @@ public class RuntimeChildChoiceDefinition extends BaseRuntimeDeclaredChildDefini
 			}
 
 			// I don't see how elementName could be null here, but eclipse complains..
-			if (elementName != null) {
+			if (!theIsSpecilization) {
-				if (myNameToChildDefinition.containsKey(elementName) == false || !nonPreferred) {
+				if (elementName != null) {
+					if (!myNameToChildDefinition.containsKey(elementName) || !nonPreferred) {
+						myNameToChildDefinition.put(elementName, nextDef);
+					}
+				}
+
+				/*
+				 * If this is a resource reference, the element name is "fooNameReference"
+				 */
+				if (IBaseResource.class.isAssignableFrom(next) || IBaseReference.class.isAssignableFrom(next)) {
+					next = theContext.getVersion().getResourceReferenceType();
+					elementName = getElementName() + myReferenceSuffix;
 					myNameToChildDefinition.put(elementName, nextDef);
 				}
 			}
 
-			/*
-			 * If this is a resource reference, the element name is "fooNameReference"
-			 */
-			if (IBaseResource.class.isAssignableFrom(next) || IBaseReference.class.isAssignableFrom(next)) {
-				next = theContext.getVersion().getResourceReferenceType();
-				elementName = getElementName() + myReferenceSuffix;
-				myNameToChildDefinition.put(elementName, nextDef);
-			}
-
 			myDatatypeToElementDefinition.put(next, nextDef);
 
 			if (myDatatypeToElementName.containsKey(next)) {
@@ -175,11 +201,6 @@ public class RuntimeChildChoiceDefinition extends BaseRuntimeDeclaredChildDefini
 				myDatatypeToElementName.put(next, elementName);
 			}
 		}
-
-		myNameToChildDefinition = Collections.unmodifiableMap(myNameToChildDefinition);
-		myDatatypeToElementName = Collections.unmodifiableMap(myDatatypeToElementName);
-		myDatatypeToElementDefinition = Collections.unmodifiableMap(myDatatypeToElementDefinition);
-		myResourceTypes = Collections.unmodifiableList(myResourceTypes);
 	}
 
 	public List<Class<? extends IBaseResource>> getResourceTypes() {
@@ -188,8 +209,7 @@ public class RuntimeChildChoiceDefinition extends BaseRuntimeDeclaredChildDefini
 
 	@Override
 	public String getChildNameByDatatype(Class<? extends IBase> theDatatype) {
-		String retVal = myDatatypeToElementName.get(theDatatype);
+		return myDatatypeToElementName.get(theDatatype);
-		return retVal;
 	}
 
 	@Override

hapi-fhir-base/src/main/java/ca/uhn/fhir/context/RuntimeChildDeclaredExtensionDefinition.java

@@ -82,7 +82,7 @@ public class RuntimeChildDeclaredExtensionDefinition extends RuntimeChildChoiceD
 			choiceTypes.add(theChildType);
 		}
 
-		setChoiceTypes(choiceTypes);
+		setChoiceTypes(choiceTypes, Collections.emptyList());
 	}
 
 	@Override

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_2_0/5730-fhir-terser-extension-enumeration.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 5730
+title: "Previously, using the FhirTerser to process an Extension with an Enumeration would fail.
+        This has been fixed."

hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/FhirTerserR4Test.java

@@ -58,6 +58,7 @@ import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -1515,7 +1516,24 @@ public class FhirTerserR4Test {
 		}
 	}
 
+	@Test
+	void extensionWithEnumeration() {
+		final FhirContext ctx = FhirContext.forR4();
+		final FhirTerser fhirTerser = ctx.newTerser();
 
+		final String url = "http://hl7.org/fhir/StructureDefinition/data-absent-reason]";
+		final Enumeration<Enumerations.DataAbsentReason> enumerationUnknown = new Enumeration<>(new Enumerations.DataAbsentReasonEnumFactory(), "unknown");
+		final Extension extensionUnknown = new Extension(url, enumerationUnknown);
+
+		final AtomicBoolean result = new AtomicBoolean(false);
+		final IModelVisitor2 iModelVisitor2 = (theElement, theContainingElementPath, theChildDefinitionPath, theElementDefinitionPath) -> {
+            result.set(true);
+            return true;
+        };
+
+		fhirTerser.visit(extensionUnknown, iModelVisitor2);
+		assertTrue(result.get());
+	}
 
 	private List<String> toStrings(List<StringType> theStrings) {
 		ArrayList<String> retVal = new ArrayList<>();

hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/ValidatorWrapper.java

@@ -120,7 +120,7 @@ class ValidatorWrapper {
 		try {
 			v = new InstanceValidator(theWorkerContext, evaluationCtx, xverManager);
 		} catch (Exception e) {
-			throw new ConfigurationException(Msg.code(648) + e);
+			throw new ConfigurationException(Msg.code(648) + e.getMessage(), e);
 		}
 
 		v.setAssumeValidRestReferences(isAssumeValidRestReferences());