From 7eb1bd14d15294d08e0163ee82840daa3c115c3a Mon Sep 17 00:00:00 2001 From: Tadgh Date: Tue, 8 Mar 2022 14:22:04 -0800 Subject: [PATCH] Do not validate auto-created placeholder resources (#3461) * Add implementation, testing, and changelog * add jira ref --- ...y-validation-on-autocreated-resources.yaml | 8 +++ .../ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java | 1 + ...sitoryValidatingInterceptorHttpR4Test.java | 24 +++++++++ ...RepositoryValidatingInterceptorR4Test.java | 50 +++++++++++++++---- .../RepositoryValidatingInterceptor.java | 30 ++++++++--- 5 files changed, 96 insertions(+), 17 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3460-skip-repository-validation-on-autocreated-resources.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3460-skip-repository-validation-on-autocreated-resources.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3460-skip-repository-validation-on-autocreated-resources.yaml new file mode 100644 index 00000000000..86695ce5585 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_0_0/3460-skip-repository-validation-on-autocreated-resources.yaml @@ -0,0 +1,8 @@ +--- +type: change +issue: 3460 +jira: SMILE-3863 +title: "Previously, during RepositoryValidation, we would perform validation on resources created via DaoConfig's `setAutoCreatePlaceholderReferenceTargets` property. This caused validation failures on placeholder resources +as they do not conform to any profile. This has been changed, and Repository Validation will not occur on any resource containing an extension with URL http://hapifhir.io/fhir/StructureDefinition/resource-placeholder`." + + diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java index 6719b675832..ae4a298f517 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java @@ -513,6 +513,7 @@ public abstract class BaseJpaR4Test extends BaseJpaTest implements ITestDataBuil myDaoConfig.setReuseCachedSearchResultsForMillis(new DaoConfig().getReuseCachedSearchResultsForMillis()); myDaoConfig.setSuppressUpdatesWithNoChange(new DaoConfig().isSuppressUpdatesWithNoChange()); myDaoConfig.setAllowContainsSearches(new DaoConfig().isAllowContainsSearches()); + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(new DaoConfig().isAutoCreatePlaceholderReferenceTargets()); myPagingProvider.setDefaultPageSize(BasePagingProvider.DEFAULT_DEFAULT_PAGE_SIZE); myPagingProvider.setMaximumPageSize(BasePagingProvider.DEFAULT_MAX_PAGE_SIZE); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorHttpR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorHttpR4Test.java index d7819f68b2d..575b72da0cd 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorHttpR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorHttpR4Test.java @@ -48,6 +48,30 @@ public class RepositoryValidatingInterceptorHttpR4Test extends BaseJpaR4Test { myInterceptorRegistry.unregisterInterceptorsIf(t -> t instanceof RepositoryValidatingInterceptor); } + @Test + public void testValidationIsSkippedOnAutoCreatedPlaceholderReferencesIfConfiguredToDoSo() { + List rules = newRuleBuilder() + .forResourcesOfType("Observation") + .requireValidationToDeclaredProfiles() + .build(); + myValInterceptor.setRules(rules); + + Observation obs = new Observation(); + obs.getCode().addCoding().setSystem("http://foo").setCode("123").setDisplay("help im a bug"); + obs.setStatus(Observation.ObservationStatus.AMENDED); + + MethodOutcome outcome = myRestfulServerExtension + .getFhirClient() + .create() + .resource(obs) + .prefer(PreferReturnEnum.OPERATION_OUTCOME) + .execute(); + + String operationOutcomeEncoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome.getOperationOutcome()); + ourLog.info("Outcome: {}", operationOutcomeEncoded); + assertThat(operationOutcomeEncoded, containsString("All observations should have a subject")); + + } @Test public void testValidationOutcomeAddedToRequestResponse() { List rules = newRuleBuilder() diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorR4Test.java index ade97f8e893..ae084849e35 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorR4Test.java @@ -6,14 +6,19 @@ import ca.uhn.fhir.rest.api.PatchTypeEnum; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; import ca.uhn.fhir.validation.ResultSeverityEnum; import org.hl7.fhir.instance.model.api.IIdType; +import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.CanonicalType; import org.hl7.fhir.r4.model.CodeType; +import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.IntegerType; import org.hl7.fhir.r4.model.Meta; import org.hl7.fhir.r4.model.Observation; import org.hl7.fhir.r4.model.OperationOutcome; +import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Parameters; import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.PractitionerRole; +import org.hl7.fhir.r4.model.Reference; import org.hl7.fhir.r4.model.StringType; import org.hl7.fhir.r5.utils.validation.constants.BestPracticeWarningLevel; import org.junit.jupiter.api.AfterEach; @@ -43,7 +48,6 @@ public class RepositoryValidatingInterceptorR4Test extends BaseJpaR4Test { myValInterceptor = new RepositoryValidatingInterceptor(); myValInterceptor.setFhirContext(myFhirContext); myInterceptorRegistry.registerInterceptor(myValInterceptor); - } @AfterEach @@ -99,7 +103,6 @@ public class RepositoryValidatingInterceptorR4Test extends BaseJpaR4Test { patient.getMeta().addProfile("http://foo/Profile1"); patient.getMeta().addProfile("http://foo/Profile9999"); myPatientDao.create(patient); - } @Test @@ -244,19 +247,23 @@ public class RepositoryValidatingInterceptorR4Test extends BaseJpaR4Test { } @Test - public void testRequireValidation_Allowed() { + public void testRequireValidationDoesNotApplyToPlaceholders() { + + //Given + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); List rules = newRuleBuilder() - .forResourcesOfType("Observation") + .forResourcesOfType("Organization") .requireValidationToDeclaredProfiles() - .withBestPracticeWarningLevel("IGNORE") .build(); myValInterceptor.setRules(rules); - Observation obs = new Observation(); - obs.getCode().addCoding().setSystem("http://foo").setCode("123").setDisplay("help im a bug"); - obs.setStatus(Observation.ObservationStatus.AMENDED); + //When + PractitionerRole pr = new PractitionerRole(); + pr.setOrganization(new Reference("Organization/400-40343834-7383-54b4-abfe-95281da21062-ProviderOrganiz")); + + //Then try { - IIdType id = myObservationDao.create(obs).getId(); + IIdType id = myPractitionerRoleDao.create(pr).getId(); assertEquals("1", id.getVersionIdPart()); } catch (PreconditionFailedException e) { // should not happen @@ -264,6 +271,31 @@ public class RepositoryValidatingInterceptorR4Test extends BaseJpaR4Test { } } + @Test + public void testRequireAtLeastProfilesDoesNotApplyToPlaceholders() { + //Given + myDaoConfig.setAutoCreatePlaceholderReferenceTargets(true); + List rules = newRuleBuilder() + .forResourcesOfType("Organization") + .requireAtLeastOneProfileOf("http://example.com/profile1", "http://example.com/profile2") + .build(); + myValInterceptor.setRules(rules); + + //When + PractitionerRole pr = new PractitionerRole(); + pr.setOrganization(new Reference("Organization/400-40343834-7383-54b4-abfe-95281da21062-ProviderOrganiz")); + + //Then + try { + IIdType id = myPractitionerRoleDao.create(pr).getId(); + assertEquals("1", id.getVersionIdPart()); + } catch (PreconditionFailedException e) { + // should not happen + fail(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(e.getOperationOutcome())); + } + } + + @Test public void testRequireValidation_AdditionalOptions() { List rules = newRuleBuilder() diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptor.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptor.java index 3a0c3eb32a6..f6b6ad12995 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptor.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptor.java @@ -27,6 +27,7 @@ import ca.uhn.fhir.interceptor.api.Interceptor; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; +import ca.uhn.fhir.util.ExtensionUtil; import ca.uhn.fhir.util.OperationOutcomeUtil; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Multimap; @@ -40,6 +41,8 @@ import java.util.Collection; import java.util.List; import java.util.stream.Collectors; +import static ca.uhn.fhir.util.HapiExtensions.EXT_RESOURCE_PLACEHOLDER; + /** * This interceptor enforces validation rules on any data saved in a HAPI FHIR JPA repository. * See Repository Validating Interceptor @@ -128,16 +131,27 @@ public class RepositoryValidatingInterceptor { } private void handle(RequestDetails theRequestDetails, IBaseResource theNewResource) { + Validate.notNull(myFhirContext, "No FhirContext has been set for this interceptor of type: %s", getClass()); - - String resourceType = myFhirContext.getResourceType(theNewResource); - Collection rules = myRules.get(resourceType); - for (IRepositoryValidatingRule nextRule : rules) { - IRepositoryValidatingRule.RuleEvaluation outcome = nextRule.evaluate(theRequestDetails, theNewResource); - if (!outcome.isPasses()) { - handleFailure(outcome); + if (!isPlaceholderResource(theNewResource)) { + String resourceType = myFhirContext.getResourceType(theNewResource); + Collection rules = myRules.get(resourceType); + for (IRepositoryValidatingRule nextRule : rules) { + IRepositoryValidatingRule.RuleEvaluation outcome = nextRule.evaluate(theRequestDetails, theNewResource); + if (!outcome.isPasses()) { + handleFailure(outcome); + } } - } + } + } + + /** + * Return true if the given resource is a placeholder resource, as identified by a specific extension + * @param theNewResource the {@link IBaseResource} to check + * @return whether or not this resource is a placeholder. + */ + private boolean isPlaceholderResource(IBaseResource theNewResource) { + return ExtensionUtil.hasExtension(theNewResource, EXT_RESOURCE_PLACEHOLDER); } protected void handleFailure(IRepositoryValidatingRule.RuleEvaluation theOutcome) {