fixed an error with concurrent validation (#3266)

* fixed an error with concurrent validation

* change log
This commit is contained in:
Ken Stevens 2021-12-29 16:23:44 -05:00 committed by GitHub
parent 1c318aac1e
commit 568d39d13a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 127 additions and 6 deletions

View File

@ -27,6 +27,7 @@ import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.util.BundleUtil; import ca.uhn.fhir.util.BundleUtil;
import ca.uhn.fhir.validation.schematron.SchematronProvider; import ca.uhn.fhir.validation.schematron.SchematronProvider;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.Validate;
import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseBundle; 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.Collectors;
import java.util.stream.IntStream; 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.) * 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); return new ConcurrentValidationTask(entryPathPrefix, future);
}).collect(Collectors.toList()); }).collect(Collectors.toList());
List<SingleValidationMessage> validationMessages = new ArrayList<>(); List<SingleValidationMessage> validationMessages = buildValidationMessages(validationTasks);
return new ValidationResult(myContext, validationMessages);
}
static List<SingleValidationMessage> buildValidationMessages(List<ConcurrentValidationTask> validationTasks) {
List<SingleValidationMessage> retval = new ArrayList<>();
try { try {
for (ConcurrentValidationTask validationTask : validationTasks) { for (ConcurrentValidationTask validationTask : validationTasks) {
ValidationResult result = validationTask.getFuture().get(); ValidationResult result = validationTask.getFuture().get();
final String bundleEntryPathPrefix = validationTask.getResourcePathPrefix(); final String bundleEntryPathPrefix = validationTask.getResourcePathPrefix();
List<SingleValidationMessage> messages = result.getMessages().stream() List<SingleValidationMessage> messages = result.getMessages().stream()
.map(message -> { .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); message.setLocationString(bundleEntryPathPrefix + currentPath);
return message; return message;
}) })
.collect(Collectors.toList()); .collect(Collectors.toList());
validationMessages.addAll(messages); retval.addAll(messages);
} }
} catch (InterruptedException | ExecutionException exp) { } catch (InterruptedException | ExecutionException exp) {
throw new InternalErrorException(exp); throw new InternalErrorException(exp);
} }
return new ValidationResult(myContext, new ArrayList<>(validationMessages)); return retval;
} }
private ValidationResult validateResource(IValidationContext<IBaseResource> theValidationContext) { private ValidationResult validateResource(IValidationContext<IBaseResource> theValidationContext) {
@ -353,11 +375,11 @@ public class FhirValidator {
} }
// Simple Tuple to keep track of bundle path and associate aync future task // 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 String myResourcePathPrefix;
private final Future<ValidationResult> myFuture; private final Future<ValidationResult> myFuture;
private ConcurrentValidationTask(String theResourcePathPrefix, Future<ValidationResult> theFuture) { ConcurrentValidationTask(String theResourcePathPrefix, Future<ValidationResult> theFuture) {
myResourcePathPrefix = theResourcePathPrefix; myResourcePathPrefix = theResourcePathPrefix;
myFuture = theFuture; myFuture = theFuture;
} }

View File

@ -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<FhirValidator.ConcurrentValidationTask> tasks = buildTasks("patient.name.first");
// execute
List<SingleValidationMessage> 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<FhirValidator.ConcurrentValidationTask> tasks = buildTasks("patient.name");
// execute
List<SingleValidationMessage> 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<FhirValidator.ConcurrentValidationTask> tasks = buildTasks(null);
// execute
List<SingleValidationMessage> 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<FhirValidator.ConcurrentValidationTask> tasks = buildTasks("patient");
// execute
List<SingleValidationMessage> resultMessages = FhirValidator.buildValidationMessages(tasks);
// validate
assertThat(resultMessages, hasSize(1));
assertEquals(MESSAGE, resultMessages.get(0).getMessage());
assertEquals(PREFIX + ".patient", resultMessages.get(0).getLocationString());
}
private List<FhirValidator.ConcurrentValidationTask> buildTasks(String theLocation) {
SingleValidationMessage message = new SingleValidationMessage();
message.setMessage(MESSAGE);
message.setLocationString(theLocation);
ValidationResult result = new ValidationResult(myFhirContext, Collections.singletonList(message));
CompletableFuture<ValidationResult> future = new CompletableFuture<>();
future.complete(result);
List<FhirValidator.ConcurrentValidationTask> tasks = new ArrayList<>();
FhirValidator.ConcurrentValidationTask task = new FhirValidator.ConcurrentValidationTask(PREFIX, future);
tasks.add(task);
return tasks;
}
}

View File

@ -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."