diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/FhirValidator.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/FhirValidator.java index fde706d9f99..4d7b8ff161b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/FhirValidator.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/validation/FhirValidator.java @@ -27,6 +27,7 @@ import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.validation.schematron.SchematronProvider; +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.IBaseBundle; @@ -44,6 +45,8 @@ import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; +import static org.apache.commons.lang3.StringUtils.isBlank; + /** * Resource validator, which checks resources for compliance against various validation schemes (schemas, schematrons, profiles, etc.) @@ -277,24 +280,43 @@ public class FhirValidator { return new ConcurrentValidationTask(entryPathPrefix, future); }).collect(Collectors.toList()); - List validationMessages = new ArrayList<>(); + List validationMessages = buildValidationMessages(validationTasks); + return new ValidationResult(myContext, validationMessages); + } + + static List buildValidationMessages(List validationTasks) { + List retval = new ArrayList<>(); try { for (ConcurrentValidationTask validationTask : validationTasks) { ValidationResult result = validationTask.getFuture().get(); final String bundleEntryPathPrefix = validationTask.getResourcePathPrefix(); List messages = result.getMessages().stream() .map(message -> { - String currentPath = message.getLocationString().substring(message.getLocationString().indexOf('.')); + String currentPath; + + String locationString = StringUtils.defaultIfEmpty(message.getLocationString(), ""); + + int dotIndex = locationString.indexOf('.'); + if (dotIndex >= 0) { + currentPath = locationString.substring(dotIndex); + } else { + if (isBlank(bundleEntryPathPrefix) || isBlank(locationString)) { + currentPath = locationString; + } else { + currentPath = "." + locationString; + } + } + message.setLocationString(bundleEntryPathPrefix + currentPath); return message; }) .collect(Collectors.toList()); - validationMessages.addAll(messages); + retval.addAll(messages); } } catch (InterruptedException | ExecutionException exp) { throw new InternalErrorException(exp); } - return new ValidationResult(myContext, new ArrayList<>(validationMessages)); + return retval; } private ValidationResult validateResource(IValidationContext theValidationContext) { @@ -353,11 +375,11 @@ public class FhirValidator { } // Simple Tuple to keep track of bundle path and associate aync future task - private static class ConcurrentValidationTask { + static class ConcurrentValidationTask { private final String myResourcePathPrefix; private final Future myFuture; - private ConcurrentValidationTask(String theResourcePathPrefix, Future theFuture) { + ConcurrentValidationTask(String theResourcePathPrefix, Future theFuture) { myResourcePathPrefix = theResourcePathPrefix; myFuture = theFuture; } diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/validation/FhirValidatorTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/validation/FhirValidatorTest.java new file mode 100644 index 00000000000..5289e890eae --- /dev/null +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/validation/FhirValidatorTest.java @@ -0,0 +1,95 @@ +package ca.uhn.fhir.validation; + +import ca.uhn.fhir.context.FhirContext; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CompletableFuture; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; +import static org.junit.jupiter.api.Assertions.assertEquals; + +@ExtendWith(MockitoExtension.class) +class FhirValidatorTest { + private static final String PREFIX = "Brakebills"; + public static final String MESSAGE = "Fillory"; + @Mock + FhirContext myFhirContext; + + @Test + public void testBuildMessagesThreePartLocation() { + // setup + List tasks = buildTasks("patient.name.first"); + + // execute + List resultMessages = FhirValidator.buildValidationMessages(tasks); + + // validate + assertThat(resultMessages, hasSize(1)); + assertEquals(MESSAGE, resultMessages.get(0).getMessage()); + assertEquals(PREFIX + ".name.first", resultMessages.get(0).getLocationString()); + } + + @Test + public void testBuildMessagesTwoPartLocation() { + // setup + List tasks = buildTasks("patient.name"); + + // execute + List resultMessages = FhirValidator.buildValidationMessages(tasks); + + // validate + assertThat(resultMessages, hasSize(1)); + assertEquals(MESSAGE, resultMessages.get(0).getMessage()); + assertEquals(PREFIX + ".name", resultMessages.get(0).getLocationString()); + } + + @Test + public void testBuildMessagesNullLocation() { + // setup + List tasks = buildTasks(null); + + // execute + List resultMessages = FhirValidator.buildValidationMessages(tasks); + + // validate + assertThat(resultMessages, hasSize(1)); + assertEquals(MESSAGE, resultMessages.get(0).getMessage()); + assertEquals(PREFIX, resultMessages.get(0).getLocationString()); + } + + @Test + public void testBuildMessagesOnePartLocation() { + // setup + List tasks = buildTasks("patient"); + + // execute + List resultMessages = FhirValidator.buildValidationMessages(tasks); + + // validate + assertThat(resultMessages, hasSize(1)); + assertEquals(MESSAGE, resultMessages.get(0).getMessage()); + assertEquals(PREFIX + ".patient", resultMessages.get(0).getLocationString()); + } + + private List buildTasks(String theLocation) { + SingleValidationMessage message = new SingleValidationMessage(); + message.setMessage(MESSAGE); + message.setLocationString(theLocation); + ValidationResult result = new ValidationResult(myFhirContext, Collections.singletonList(message)); + CompletableFuture future = new CompletableFuture<>(); + future.complete(result); + List tasks = new ArrayList<>(); + FhirValidator.ConcurrentValidationTask task = new FhirValidator.ConcurrentValidationTask(PREFIX, future); + tasks.add(task); + return tasks; + } + + +} diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3266-concurrent-validation.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3266-concurrent-validation.yaml new file mode 100644 index 00000000000..f189fce21be --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3266-concurrent-validation.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 3266 +title: "Concurrent validation failed with StringIndexOutOfBoundsException when the validation location did not contain a '.'. This has been corrected."