Fixing concurrency issue in WorkerContextValidationSupportAdapter (#6554) (#6648)

* Fixing concurrency issue in VersionSpecificWorkerContextWrapper (#6554)

When validating if a code is valid against a ValueSet, a potentially
cached (and therefore shared) result object was mutated. This mutation
is now done on a copy of the result object.

* Added test case for concurrent validateCode with issue in VS and CS

The added test case checks if concurrent validation of a code, that
is neither in the ValueSet nor in the CodeSystem creates a validation
result with the CS issue added to the validation result correctly.

* Credit for #6648

---------

Co-authored-by: James Agnew <[email protected]>

Author: Boereck

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/8_6_0/6648-resolve-concurrency-issue-in-code-validation.yaml

@@ -0,0 +1,5 @@
+---
+type: fix
+issue: 6648
+title: "When performing validation with high concurrency, code validations could
+fail with a ConcurrentModificationException. Thanks to Max Bureck for the contribution!"

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

@@ -5,6 +5,8 @@
 import ca.uhn.fhir.context.support.ConceptValidationOptions;
 import ca.uhn.fhir.context.support.DefaultProfileValidationSupport;
 import ca.uhn.fhir.context.support.IValidationSupport;
+import ca.uhn.fhir.context.support.IValidationSupport.BaseConceptProperty;
+import ca.uhn.fhir.context.support.IValidationSupport.CodeValidationIssue;
 import ca.uhn.fhir.context.support.ValidationSupportContext;
 import ca.uhn.fhir.i18n.Msg;
 import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException;
@@ -898,6 +900,7 @@ private IValidationSupport.CodeValidationResult validateCodeInValueSet(
 			final boolean valueSetResultContainsInvalidDisplay = result.getIssues().stream()
 					.anyMatch(WorkerContextValidationSupportAdapter::hasInvalidDisplayDetailCode);
 			if (codeSystemResult != null) {
+				result = copyCodeValidationResult(result);
 				for (IValidationSupport.CodeValidationIssue codeValidationIssue : codeSystemResult.getIssues()) {
 					/* Value set validation should already have checked the display name. If we get INVALID_DISPLAY
 					issues from code system validation, they will only repeat what was already caught.
@@ -911,6 +914,33 @@ private IValidationSupport.CodeValidationResult validateCodeInValueSet(
 		return result;
 	}
 
+	private IValidationSupport.CodeValidationResult copyCodeValidationResult(
+			IValidationSupport.CodeValidationResult toCopy) {
+		IValidationSupport.CodeValidationResult result = new IValidationSupport.CodeValidationResult();
+
+		List<CodeValidationIssue> issues = toCopy.getIssues();
+		List<BaseConceptProperty> properties = toCopy.getProperties();
+
+		result.setCode(toCopy.getCode())
+				.setCodeSystemName(toCopy.getCodeSystemName())
+				.setCodeSystemVersion(toCopy.getCodeSystemVersion())
+				.setDisplay(toCopy.getDisplay())
+				.setIssues(copyList(issues))
+				.setMessage(toCopy.getMessage())
+				.setSeverity(toCopy.getSeverity())
+				.setSourceDetails(toCopy.getSourceDetails())
+				.setProperties(copyList(properties));
+		return result;
+	}
+
+	private <T> List<T> copyList(List<T> issues) {
+		if (issues == null) {
+			return null;
+		} else {
+			return new ArrayList<>(issues);
+		}
+	}
+
 	@Nonnull
 	private ValidationSupportContext newValidationSupportContext() {
 		return new ValidationSupportContext(myValidationSupport);

hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/WorkerContextValidationSupportAdapterTest.java

@@ -3,16 +3,26 @@
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.support.DefaultProfileValidationSupport;
 import ca.uhn.fhir.context.support.IValidationSupport;
+import ca.uhn.fhir.context.support.IValidationSupport.CodeValidationIssue;
+import ca.uhn.fhir.context.support.IValidationSupport.CodeValidationIssueCode;
+import ca.uhn.fhir.context.support.IValidationSupport.CodeValidationIssueCoding;
+import ca.uhn.fhir.context.support.IValidationSupport.CodeValidationResult;
+import ca.uhn.fhir.context.support.IValidationSupport.IssueSeverity;
 import ca.uhn.fhir.context.support.ValidationSupportContext;
 import ca.uhn.fhir.fhirpath.BaseValidationTestWithInlineMocks;
 import org.hl7.fhir.r4.model.StructureDefinition;
+import org.hl7.fhir.r5.model.Coding;
 import org.hl7.fhir.r5.model.PackageInformation;
 import org.hl7.fhir.r5.model.Resource;
 import org.hl7.fhir.r5.model.ValueSet;
+import org.hl7.fhir.r5.terminologies.utilities.ValidationResult;
 import org.hl7.fhir.utilities.validation.ValidationOptions;
 import org.junit.jupiter.api.Test;
 
+import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.ForkJoinTask;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -109,6 +119,45 @@ public void validateCode_codeNotInValueSet_doesNotResolveSystem() {
 		verify(myValidationSupport, never()).validateCode(any(), any(), any(), any(), any(), any());
 	}
 
+	@Test
+	public void validateCode_codeNotInValueSet_codeNotInSystem_concurrent() {
+		// setup
+		setupValidation();
+
+		String system = "http://codesystems.com/system";
+		String badCode = "code3";
+
+		CodeValidationResult codeInValuesetValResult = new CodeValidationResult().setCode(badCode).setCodeSystemName(system).setMessage("Bad code in VS");
+		when(myValidationSupport.validateCodeInValueSet(any(), any(), eq(system), eq(badCode), any(), any())).thenReturn(codeInValuesetValResult);
+
+		// We expect this code validation result as an additional result on the validation, if code is in value set
+		String issueMessage = "Code not in here!";
+		CodeValidationIssue codeValidationIssue = new CodeValidationIssue(issueMessage, IssueSeverity.ERROR, CodeValidationIssueCode.NOT_FOUND, CodeValidationIssueCoding.NOT_FOUND);
+		CodeValidationResult codeValResult = new CodeValidationResult().setMessage("Bad code in CS").setCode(badCode).setCodeSystemName(system).setSeverity(IssueSeverity.ERROR).addIssue(codeValidationIssue);
+		when(myValidationSupport.validateCode(any(), any(), eq("http://codesystems.com/system"), eq(badCode) , any(), eq(null))).thenReturn(codeValResult);
+
+		ForkJoinPool pool = ForkJoinPool.commonPool();
+		List<ForkJoinTask<?>> futures = new ArrayList<>();
+		for(int i=0; i<1000; i++) {
+			ForkJoinTask<?> f = pool.submit(() -> {
+				org.hl7.fhir.r5.model.ValueSet valueSet = new ValueSet();
+				valueSet.getCompose().addInclude().setSystem(system).addConcept().setCode("code0");
+				valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system2").addConcept().setCode("code2");
+
+				ValidationResult result = myWorkerContextWrapper.validateCode(new ValidationOptions(), new Coding(system, badCode, ""), valueSet);
+				
+				// here we check if the additional code validation result was added
+				assertThat(result.getIssues()).hasSize(1);
+				assertThat(result.getIssues().get(0).getDiagnostics()).isEqualTo(issueMessage);
+			});
+			futures.add(f);
+		}
+		// test if there was no assertion exception for any invocation
+		for(ForkJoinTask<?> f : futures) {
+			f.join();
+		}
+	}
+
 	@Test
 	public void isPrimitive_primitive() {
 		// setup