diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java index 22af958ae56..c79b18fd9ae 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleUtil.java @@ -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])); } diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java index 23c4130ee2e..0ba1e642895 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java @@ -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 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 e87d5e64729..e8f89ba7e16 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 @@ -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 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 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 future = myExecutorService.submit(() -> { - IValidationContext entryValidationContext = ValidationContext.forResource(theValidationContext.getFhirContext(), entry, theOptions); + IValidationContext 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 buildValidationMessages(List validationTasks) { List 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; diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3327-concurrent-validation-perf.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3327-concurrent-validation-perf.yaml new file mode 100644 index 00000000000..dc035894c5f --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_7_0/3327-concurrent-validation-perf.yaml @@ -0,0 +1,5 @@ +--- +type: perf +issue: 3327 +jira: SMILE-3438 +title: "Improved performance of validating resources when skip contained resources is enabled." diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java index e9befcb0cd4..bf5ebad9273 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java @@ -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()); } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/validation/FhirValidatorTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/validation/FhirValidatorTest.java new file mode 100644 index 00000000000..9d708583f04 --- /dev/null +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/validation/FhirValidatorTest.java @@ -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()); + } +}