concurrent validation performance improvement (#3327)

This commit is contained in:
Ken Stevens 2022-01-23 15:55:31 -05:00 committed by GitHub
parent 42ff607561
commit ed2ea91f56
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 89 additions and 14 deletions

View File

@ -13,7 +13,6 @@ import ca.uhn.fhir.util.bundle.BundleEntryParts;
import ca.uhn.fhir.util.bundle.EntryListAccumulator;
import ca.uhn.fhir.util.bundle.ModifiableBundleEntry;
import ca.uhn.fhir.util.bundle.SearchBundleEntryParts;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.hl7.fhir.instance.model.api.IBase;
import org.hl7.fhir.instance.model.api.IBaseBinary;
@ -27,7 +26,6 @@ import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
import java.util.stream.Collectors;
@ -231,7 +229,7 @@ public class BundleUtil {
retVal.addAll(partsToIBaseMap.values());
//Blow away the entries and reset them in the right order.
TerserUtil.clearField(theContext, "entry", theBundle);
TerserUtil.clearField(theContext, theBundle, "entry");
TerserUtil.setField(theContext, "entry", theBundle, retVal.toArray(new IBase[0]));
}

View File

@ -315,12 +315,11 @@ public final class TerserUtil {
/**
* Clears the specified field on the resource provided
*
* @param theFhirContext Context holding resource definition
* @param theFieldName
* @param theFhirContext Context holding resource definition
* @param theResource
* @param theFieldName
*/
public static void clearField(FhirContext theFhirContext, String theFieldName, IBaseResource theResource) {
public static void clearField(FhirContext theFhirContext, IBaseResource theResource, String theFieldName) {
BaseRuntimeChildDefinition childDefinition = getBaseRuntimeChildDefinition(theFhirContext, theFieldName, theResource);
clear(childDefinition.getAccessor().getValues(theResource));
}
@ -341,7 +340,7 @@ public final class TerserUtil {
/**
* Sets the provided field with the given values. This method will add to the collection of existing field values
* in case of multiple cardinality. Use {@link #clearField(FhirContext, String, IBaseResource)}
* in case of multiple cardinality. Use {@link #clearField(FhirContext, IBaseResource, String)}
* to remove values before setting
*
* @param theFhirContext Context holding resource definition
@ -355,7 +354,7 @@ public final class TerserUtil {
/**
* Sets the provided field with the given values. This method will add to the collection of existing field values
* in case of multiple cardinality. Use {@link #clearField(FhirContext, String, IBaseResource)}
* in case of multiple cardinality. Use {@link #clearField(FhirContext, IBaseResource, String)}
* to remove values before setting
*
* @param theFhirContext Context holding resource definition

View File

@ -26,10 +26,10 @@ import ca.uhn.fhir.interceptor.api.IInterceptorBroadcaster;
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.util.TerserUtil;
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;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.slf4j.Logger;
@ -40,7 +40,6 @@ import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
@ -70,6 +69,7 @@ public class FhirValidator {
private List<IValidatorModule> myValidators = new ArrayList<>();
private IInterceptorBroadcaster myInterceptorBroadcaster;
private boolean myConcurrentBundleValidation;
private boolean mySkipContainedReferenceValidation;
private ExecutorService myExecutorService;
@ -271,10 +271,18 @@ public class FhirValidator {
// Async validation tasks
List<ConcurrentValidationTask> validationTasks = IntStream.range(0, entries.size())
.mapToObj(index -> {
IBaseResource resourceToValidate;
IBaseResource entry = entries.get(index);
String entryPathPrefix = String.format("Bundle.entry[%d].resource.ofType(%s)", index, entry.fhirType());
if (mySkipContainedReferenceValidation) {
resourceToValidate = withoutContainedResources(entry);
} else {
resourceToValidate = entry;
}
String entryPathPrefix = String.format("Bundle.entry[%d].resource.ofType(%s)", index, resourceToValidate.fhirType());
Future<ValidationResult> future = myExecutorService.submit(() -> {
IValidationContext<IBaseResource> entryValidationContext = ValidationContext.forResource(theValidationContext.getFhirContext(), entry, theOptions);
IValidationContext<IBaseResource> entryValidationContext = ValidationContext.forResource(theValidationContext.getFhirContext(), resourceToValidate, theOptions);
return validateResource(entryValidationContext);
});
return new ConcurrentValidationTask(entryPathPrefix, future);
@ -284,6 +292,16 @@ public class FhirValidator {
return new ValidationResult(myContext, validationMessages);
}
IBaseResource withoutContainedResources(IBaseResource theEntry) {
if (TerserUtil.hasValues(myContext, theEntry, "contained")) {
IBaseResource deepCopy = TerserUtil.clone(myContext, theEntry);
TerserUtil.clearField(myContext, deepCopy, "contained");
return deepCopy;
} else {
return theEntry;
}
}
static List<SingleValidationMessage> buildValidationMessages(List<ConcurrentValidationTask> validationTasks) {
List<SingleValidationMessage> retval = new ArrayList<>();
try {
@ -374,6 +392,23 @@ public class FhirValidator {
return this;
}
/**
* If this is true, any resource that has contained resources will first be deep-copied and then the contained
* resources remove from the copy and this copy without contained resources will be validated.
*/
public boolean isSkipContainedReferenceValidation() {
return mySkipContainedReferenceValidation;
}
/**
* If this is true, any resource that has contained resources will first be deep-copied and then the contained
* resources remove from the copy and this copy without contained resources will be validated.
*/
public FhirValidator setSkipContainedReferenceValidation(boolean theSkipContainedReferenceValidation) {
mySkipContainedReferenceValidation = theSkipContainedReferenceValidation;
return this;
}
// Simple Tuple to keep track of bundle path and associate aync future task
static class ConcurrentValidationTask {
private final String myResourcePathPrefix;

View File

@ -0,0 +1,5 @@
---
type: perf
issue: 3327
jira: SMILE-3438
title: "Improved performance of validating resources when skip contained resources is enabled."

View File

@ -460,7 +460,7 @@ class TerserUtilTest {
Patient p1 = new Patient();
p1.addName().setFamily("Doe");
TerserUtil.clearField(ourFhirContext, "name", p1);
TerserUtil.clearField(ourFhirContext, p1, "name");
assertEquals(0, p1.getName().size());
}

View File

@ -0,0 +1,38 @@
package ca.uhn.fhir.validation;
import ca.uhn.fhir.context.FhirContext;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.StringType;
import org.junit.jupiter.api.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class FhirValidatorTest {
FhirContext ourFhirContext = FhirContext.forR4Cached();
@Test
public void testWithoutContainedResourcesDoesNotAlter() {
// setup
FhirValidator validator = new FhirValidator(ourFhirContext);
Patient patient = new Patient();
patient.addName().addGiven("bob");
Observation obs = new Observation();
obs.setValue(new StringType("heavy"));
patient.getContained().add(obs);
// run
Patient patientWithoutContained = (Patient)validator.withoutContainedResources(patient);
// check
assertEquals("bob", patientWithoutContained.getNameFirstRep().getGivenAsSingleString());
assertThat(patientWithoutContained.getContained(), hasSize(0));
assertEquals("bob", patient.getNameFirstRep().getGivenAsSingleString());
assertThat(patient.getContained(), hasSize(1));
assertEquals("heavy", ((Observation)patient.getContained().get(0)).getValue().toString());
}
}