Repository validation enhancements (#2270)

This commit is contained in:
James Agnew 2021-01-05 18:22:34 -05:00 committed by GitHub
parent 12d366c86e
commit 0bb878e4a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 211 additions and 6 deletions

View File

@ -25,6 +25,7 @@ import ca.uhn.fhir.interceptor.executor.InterceptorService;
import ca.uhn.fhir.jpa.interceptor.validation.IRepositoryValidatingRule;
import ca.uhn.fhir.jpa.interceptor.validation.RepositoryValidatingInterceptor;
import ca.uhn.fhir.jpa.interceptor.validation.RepositoryValidatingRuleBuilder;
import ca.uhn.fhir.validation.ResultSeverityEnum;
import org.springframework.context.ApplicationContext;
import java.util.List;
@ -88,6 +89,28 @@ public class RepositoryValidatingInterceptorExamples {
//END SNIPPET: requireValidationToDeclaredProfiles
}
public void requireValidationToDeclaredProfilesAdjustThreshold() {
RepositoryValidatingRuleBuilder ruleBuilder = myAppCtx.getBean(RepositoryValidatingRuleBuilder.class);
//START SNIPPET: requireValidationToDeclaredProfilesAdjustThreshold
ruleBuilder
.forResourcesOfType("Patient")
.requireValidationToDeclaredProfiles()
.rejectOnSeverity(ResultSeverityEnum.WARNING);
//END SNIPPET: requireValidationToDeclaredProfilesAdjustThreshold
}
public void requireValidationToDeclaredProfilesTagOnFailure() {
RepositoryValidatingRuleBuilder ruleBuilder = myAppCtx.getBean(RepositoryValidatingRuleBuilder.class);
//START SNIPPET: requireValidationToDeclaredProfilesTagOnFailure
ruleBuilder
.forResourcesOfType("Patient")
.requireValidationToDeclaredProfiles()
.dontReject()
.tagOnSeverity(ResultSeverityEnum.ERROR, "http://example.com", "validation-failure");
//END SNIPPET: requireValidationToDeclaredProfilesTagOnFailure
}
public void disallowProfiles() {
RepositoryValidatingRuleBuilder ruleBuilder = myAppCtx.getBean(RepositoryValidatingRuleBuilder.class);

View File

@ -76,6 +76,24 @@ This rule is generally combined with the *Require Profile Declarations* above.
Any resource creates or updates that do not conform to the given profile will be rejected.
## Adjusting Failure Threshold
By default, any validation messages with a severity value of *ERROR* or *FATAL* will result in resource creates or updates being rejected. This threshold can be adjusted however:
```java
{{snippet:classpath:/ca/uhn/hapi/fhir/docs/RepositoryValidatingInterceptorExamples.java|requireValidationToDeclaredProfilesAdjustThreshold}}
```
## Tagging Validation Failures
By default, resource updates/changes resulting in failing validation will cause the operation to be rolled back. You can alternately configure the rule to allow the change to proceed but add an arbitrary tag to the resource when it is saved.
```java
{{snippet:classpath:/ca/uhn/hapi/fhir/docs/RepositoryValidatingInterceptorExamples.java|requireValidationToDeclaredProfilesTagOnFailure}}
```
# Rules: Disallow Specific Profiles
Rules can declare that a specific profile is not allowed.

View File

@ -635,7 +635,6 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
IBaseResource oldVersion = toResource(theEntity, false);
List<TagDefinition> tags = toTagList(theMetaAdd);
for (TagDefinition nextDef : tags) {
boolean hasTag = false;
@ -663,9 +662,10 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
validateMetaCount(theEntity.getTags().size());
theEntity = myEntityManager.merge(theEntity);
myEntityManager.merge(theEntity);
// Interceptor call: STORAGE_PRECOMMIT_RESOURCE_UPDATED
// Interceptor call: STORAGE_PRESTORAGE_RESOURCE_UPDATED
IBaseResource newVersion = toResource(theEntity, false);
HookParams params = new HookParams()
.add(IBaseResource.class, oldVersion)
@ -673,6 +673,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
.add(RequestDetails.class, theRequestDetails)
.addIfMatchesType(ServletRequestDetails.class, theRequestDetails)
.add(TransactionDetails.class, theTransactionDetails);
myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, params);
myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params);
}
@ -681,6 +682,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
IBaseResource oldVersion = toResource(theEntity, false);
List<TagDefinition> tags = toTagList(theMetaDel);
for (TagDefinition nextDef : tags) {
@ -708,6 +710,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
.add(RequestDetails.class, theRequestDetails)
.addIfMatchesType(ServletRequestDetails.class, theRequestDetails)
.add(TransactionDetails.class, theTransactionDetails);
myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, params);
myInterceptorBroadcaster.callHooks(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, params);
}

View File

@ -46,4 +46,5 @@ abstract class BaseTypedRule implements IRepositoryValidatingRule {
protected FhirContext getFhirContext() {
return myFhirContext;
}
}

View File

@ -93,7 +93,7 @@ public class RepositoryValidatingInterceptor {
/**
* Interceptor hook method. This method should not be called directly.
*/
@Hook(Pointcut.STORAGE_PRECOMMIT_RESOURCE_CREATED)
@Hook(Pointcut.STORAGE_PRESTORAGE_RESOURCE_CREATED)
void create(IBaseResource theResource) {
handle(theResource);
}
@ -101,7 +101,7 @@ public class RepositoryValidatingInterceptor {
/**
* Interceptor hook method. This method should not be called directly.
*/
@Hook(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED)
@Hook(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED)
void update(IBaseResource theOldResource, IBaseResource theNewResource) {
handle(theNewResource);
}

View File

@ -23,6 +23,7 @@ package ca.uhn.fhir.jpa.interceptor.validation;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.context.support.IValidationSupport;
import ca.uhn.fhir.jpa.validation.ValidatorResourceFetcher;
import ca.uhn.fhir.validation.ResultSeverityEnum;
import org.apache.commons.lang3.Validate;
import org.apache.commons.text.WordUtils;
import org.hl7.fhir.r5.utils.IResourceValidator;
@ -34,6 +35,7 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import static com.google.common.base.Ascii.toLowerCase;
import static org.apache.commons.lang3.StringUtils.isNotBlank;
/**
@ -204,6 +206,77 @@ public final class RepositoryValidatingRuleBuilder implements IRuleRoot {
myRule.setBestPracticeWarningLevel(bestPracticeWarningLevel);
return this;
}
/**
* Specifies that the resource should not be rejected from storage even if it does not pass validation.
*/
@Nonnull
public FinalizedRequireValidationRule dontReject() {
myRule.dontReject();
return this;
}
/**
* Specifies the minimum validation result severity that should cause a rejection. For example, if
* this is set to <code>ERROR</code> (which is the default), any validation results with a severity
* of <code>ERROR</code> or <code>FATAL</code> will cause the create/update operation to be rejected and
* rolled back, and no data will be saved.
* <p>
* Valid values must be drawn from {@link ResultSeverityEnum}
* </p>
*/
@Nonnull
public FinalizedRequireValidationRule rejectOnSeverity(@Nonnull String theSeverity) {
ResultSeverityEnum severity = ResultSeverityEnum.fromCode(toLowerCase(theSeverity));
Validate.notNull(severity, "Invalid severity code: %s", theSeverity);
return rejectOnSeverity(severity);
}
/**
* Specifies the minimum validation result severity that should cause a rejection. For example, if
* this is set to <code>ERROR</code> (which is the default), any validation results with a severity
* of <code>ERROR</code> or <code>FATAL</code> will cause the create/update operation to be rejected and
* rolled back, and no data will be saved.
* <p>
* Valid values must be drawn from {@link ResultSeverityEnum}
* </p>
*/
@Nonnull
public FinalizedRequireValidationRule rejectOnSeverity(@Nonnull ResultSeverityEnum theSeverity) {
myRule.rejectOnSeverity(theSeverity);
return this;
}
/**
* Specifies that if the validation results in any results with a severity of <code>theSeverity</code> or
* greater, the resource will be tagged with the given tag when it is saved.
*
* @param theSeverity The minimum severity. Must be drawn from values in {@link ResultSeverityEnum} and must not be <code>null</code>
* @param theTagSystem The system for the tag to add. Must not be <code>null</code>
* @param theTagCode The code for the tag to add. Must not be <code>null</code>
* @return
*/
@Nonnull
public FinalizedRequireValidationRule tagOnSeverity(@Nonnull String theSeverity,@Nonnull String theTagSystem,@Nonnull String theTagCode) {
ResultSeverityEnum severity = ResultSeverityEnum.fromCode(toLowerCase(theSeverity));
return tagOnSeverity(severity, theTagSystem, theTagCode);
}
/**
* Specifies that if the validation results in any results with a severity of <code>theSeverity</code> or
* greater, the resource will be tagged with the given tag when it is saved.
*
* @param theSeverity The minimum severity. Must be drawn from values in {@link ResultSeverityEnum} and must not be <code>null</code>
* @param theTagSystem The system for the tag to add. Must not be <code>null</code>
* @param theTagCode The code for the tag to add. Must not be <code>null</code>
* @return
*/
@Nonnull
public FinalizedRequireValidationRule tagOnSeverity(@Nonnull ResultSeverityEnum theSeverity,@Nonnull String theTagSystem,@Nonnull String theTagCode) {
myRule.tagOnSeverity(theSeverity, theTagSystem, theTagCode);
return this;
}
}
}

View File

@ -27,18 +27,24 @@ import ca.uhn.fhir.validation.FhirValidator;
import ca.uhn.fhir.validation.ResultSeverityEnum;
import ca.uhn.fhir.validation.SingleValidationMessage;
import ca.uhn.fhir.validation.ValidationResult;
import org.apache.commons.lang3.Validate;
import org.hl7.fhir.common.hapi.validation.validator.FhirInstanceValidator;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.r5.utils.IResourceValidator;
import javax.annotation.Nonnull;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
class RequireValidationRule extends BaseTypedRule {
private final IValidationSupport myValidationSupport;
private final ValidatorResourceFetcher myValidatorResourceFetcher;
private final FhirInstanceValidator myValidator;
private ResultSeverityEnum myRejectOnSeverity = ResultSeverityEnum.ERROR;
private List<TagOnSeverity> myTagOnSeverity = Collections.emptyList();
RequireValidationRule(FhirContext theFhirContext, String theType, IValidationSupport theValidationSupport, ValidatorResourceFetcher theValidatorResourceFetcher) {
public RequireValidationRule(FhirContext theFhirContext, String theType, IValidationSupport theValidationSupport, ValidatorResourceFetcher theValidatorResourceFetcher) {
super(theFhirContext, theType);
myValidationSupport = theValidationSupport;
myValidatorResourceFetcher = theValidatorResourceFetcher;
@ -62,10 +68,67 @@ class RequireValidationRule extends BaseTypedRule {
for (SingleValidationMessage next : outcome.getMessages()) {
if (next.getSeverity().ordinal() >= ResultSeverityEnum.ERROR.ordinal()) {
return RuleEvaluation.forFailure(this, outcome.toOperationOutcome());
if (myRejectOnSeverity != null && myRejectOnSeverity.ordinal() <= next.getSeverity().ordinal()) {
return RuleEvaluation.forFailure(this, outcome.toOperationOutcome());
}
}
for (TagOnSeverity nextTagOnSeverity : myTagOnSeverity) {
if (next.getSeverity().ordinal() >= nextTagOnSeverity.getSeverity()) {
theResource
.getMeta()
.addTag()
.setSystem(nextTagOnSeverity.getTagSystem())
.setCode(nextTagOnSeverity.getTagCode());
}
}
}
return RuleEvaluation.forSuccess(this);
}
public void rejectOnSeverity(ResultSeverityEnum theSeverity) {
myRejectOnSeverity = theSeverity;
}
public void tagOnSeverity(ResultSeverityEnum theSeverity, String theTagSystem, String theTagCode) {
Validate.notNull(theSeverity, "theSeverity must not be null");
Validate.notEmpty(theTagSystem, "theTagSystem must not be null or empty");
Validate.notEmpty(theTagCode, "theTagCode must not be null or empty");
if (myTagOnSeverity.isEmpty()) {
myTagOnSeverity = new ArrayList<>();
}
myTagOnSeverity.add(new TagOnSeverity(theSeverity.ordinal(), theTagSystem, theTagCode));
}
public void dontReject() {
myRejectOnSeverity = null;
}
private static class TagOnSeverity {
private final int mySeverity;
private final String myTagSystem;
private final String myTagCode;
private TagOnSeverity(int theSeverity, String theTagSystem, String theTagCode) {
mySeverity = theSeverity;
myTagSystem = theTagSystem;
myTagCode = theTagCode;
}
public int getSeverity() {
return mySeverity;
}
public String getTagSystem() {
return myTagSystem;
}
public String getTagCode() {
return myTagCode;
}
}
}

View File

@ -4,6 +4,7 @@ import ca.uhn.fhir.jpa.config.BaseConfig;
import ca.uhn.fhir.jpa.dao.r4.BaseJpaR4Test;
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.CanonicalType;
import org.hl7.fhir.r4.model.CodeType;
@ -29,6 +30,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
public class RepositoryValidatingInterceptorR4Test extends BaseJpaR4Test {
@ -262,12 +264,34 @@ public class RepositoryValidatingInterceptorR4Test extends BaseJpaR4Test {
}
}
@Test
public void testRequireValidation_FailNoRejectAndTag() {
List<IRepositoryValidatingRule> rules = newRuleBuilder()
.forResourcesOfType("Observation")
.requireValidationToDeclaredProfiles()
.withBestPracticeWarningLevel("IGNORE")
.dontReject()
.tagOnSeverity(ResultSeverityEnum.ERROR, "http://foo", "validation-error")
.build();
myValInterceptor.setRules(rules);
Observation obs = new Observation();
obs.getCode().addCoding().setSystem("http://foo").setCode("123").setDisplay("help im a bug");
IIdType id = myObservationDao.create(obs).getId();
assertEquals("1", id.getVersionIdPart());
obs = myObservationDao.read(id);
assertTrue(obs.getMeta().hasTag());
assertTrue(obs.getMeta().getTag("http://foo", "validation-error") != null);
}
@Test
public void testRequireValidation_Blocked() {
List<IRepositoryValidatingRule> rules = newRuleBuilder()
.forResourcesOfType("Observation")
.requireValidationToDeclaredProfiles()
.withBestPracticeWarningLevel("IGNORE")
.rejectOnSeverity("error")
.build();
myValInterceptor.setRules(rules);