Fix validation caching issue with initially unpopulated StructureDefinitions (#5705)
* Solution to caching issue with new tests. * Fix animal sniffer issue. Clean up unit tests. Finalize changelog. Add TODO referencing change in validation cache behaviour. * Spotless. * Fix unit test. * Code review feedback.
This commit is contained in:
parent
bdaedb605b
commit
c8d6e9fb73
|
@ -0,0 +1,8 @@
|
|||
---
|
||||
type: fix
|
||||
issue: 5707
|
||||
jira: SMILE-7270
|
||||
title: "Previously, with validation active, when a user POSTed a resource with a meta profile with a non-existent
|
||||
StructureDefinition URL, then POSTed the StructureDefinition, POSTing the same or another patient with that same
|
||||
meta profile URL would still fail with a VALIDATION_VAL_PROFILE_UNKNOWN_NOT_POLICY validation error.
|
||||
This has been fixed."
|
|
@ -91,6 +91,9 @@ public class JpaPersistedResourceValidationSupport implements IValidationSupport
|
|||
// TermReadSvcImpl calls these methods as a part of its "isCodeSystemSupported" calls.
|
||||
// We should modify CachingValidationSupport to cache the results of "isXXXSupported"
|
||||
// at which point we could do away with this cache
|
||||
// TODO: LD: This cache seems to supersede the cache in CachingValidationSupport, as that cache is set to
|
||||
// 10 minutes, but this 1 minute cache now determines the expiry.
|
||||
// This new behaviour was introduced between the 7.0.0 release and the current master (7.2.0)
|
||||
private Cache<String, IBaseResource> myLoadCache = CacheFactory.build(TimeUnit.MINUTES.toMillis(1), 1000);
|
||||
|
||||
/**
|
||||
|
@ -188,6 +191,9 @@ public class JpaPersistedResourceValidationSupport implements IValidationSupport
|
|||
IBaseResource fetched = myLoadCache.get(key, t -> doFetchResource(theClass, theUri));
|
||||
|
||||
if (fetched == myNoMatch) {
|
||||
ourLog.debug(
|
||||
"Invalidating cache entry for URI: {} since the result of the underlying query is empty", theUri);
|
||||
myLoadCache.invalidate(key);
|
||||
return null;
|
||||
}
|
||||
|
||||
|
|
|
@ -600,7 +600,7 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
|
|||
fail(myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(e.getOperationOutcome()));
|
||||
}
|
||||
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
|
||||
assertEquals(12, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
|
||||
assertEquals(14, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
|
||||
assertEquals(0, myCaptureQueriesListener.getUpdateQueriesForCurrentThread().size());
|
||||
assertEquals(0, myCaptureQueriesListener.getInsertQueriesForCurrentThread().size());
|
||||
assertEquals(0, myCaptureQueriesListener.getDeleteQueriesForCurrentThread().size());
|
||||
|
@ -610,14 +610,14 @@ public class FhirResourceDaoR4QueryCountTest extends BaseResourceProviderR4Test
|
|||
myCaptureQueriesListener.clear();
|
||||
myObservationDao.validate(obs, null, null, null, null, null, null);
|
||||
myCaptureQueriesListener.logSelectQueriesForCurrentThread();
|
||||
assertEquals(0, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
|
||||
assertEquals(6, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size());
|
||||
myCaptureQueriesListener.logUpdateQueriesForCurrentThread();
|
||||
assertEquals(0, myCaptureQueriesListener.getUpdateQueriesForCurrentThread().size());
|
||||
myCaptureQueriesListener.logInsertQueriesForCurrentThread();
|
||||
assertEquals(0, myCaptureQueriesListener.getInsertQueriesForCurrentThread().size());
|
||||
myCaptureQueriesListener.logDeleteQueriesForCurrentThread();
|
||||
assertEquals(0, myCaptureQueriesListener.getDeleteQueriesForCurrentThread().size());
|
||||
assertEquals(0, myCaptureQueriesListener.getCommitCount());
|
||||
assertEquals(6, myCaptureQueriesListener.getCommitCount());
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -31,8 +31,6 @@ import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
|
|||
import ca.uhn.fhir.util.OperationOutcomeUtil;
|
||||
import ca.uhn.fhir.util.StopWatch;
|
||||
import ca.uhn.fhir.validation.IValidatorModule;
|
||||
import ca.uhn.fhir.validation.ResultSeverityEnum;
|
||||
import ca.uhn.fhir.validation.ValidationResult;
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.hl7.fhir.common.hapi.validation.support.InMemoryTerminologyServerValidationSupport;
|
||||
import org.hl7.fhir.common.hapi.validation.support.UnknownCodeSystemWarningValidationSupport;
|
||||
|
@ -49,10 +47,10 @@ import org.hl7.fhir.utilities.i18n.I18nConstants;
|
|||
import org.hl7.fhir.utilities.xhtml.XhtmlNode;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Disabled;
|
||||
import org.junit.jupiter.api.Nested;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.CsvSource;
|
||||
import org.junit.jupiter.params.provider.ValueSource;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.test.util.AopTestUtils;
|
||||
|
||||
|
@ -1985,23 +1983,76 @@ public class FhirResourceDaoR4ValidateTest extends BaseJpaR4Test {
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testValidateUsingDifferentialProfile() throws IOException {
|
||||
StructureDefinition sd = loadResourceFromClasspath(StructureDefinition.class, "/r4/profile-differential-patient-r4.json");
|
||||
myStructureDefinitionDao.create(sd);
|
||||
@Nested
|
||||
class TestValidateUsingDifferentialProfile {
|
||||
private static final String PROFILE_URL = "http://example.com/fhir/StructureDefinition/patient-1a-extensions";
|
||||
|
||||
Patient p = new Patient();
|
||||
p.getText().setStatus(Narrative.NarrativeStatus.GENERATED);
|
||||
p.getText().getDiv().setValue("<div>hello</div>");
|
||||
p.getMeta().addProfile("http://example.com/fhir/StructureDefinition/patient-1a-extensions");
|
||||
p.setActive(true);
|
||||
private static final Patient PATIENT_WITH_REAL_URL = createPatient(PROFILE_URL);
|
||||
private static final Patient PATIENT_WITH_FAKE_URL = createPatient("https://www.i.do.not.exist.com");
|
||||
|
||||
String raw = myFhirContext.newJsonParser().encodeResourceToString(p);
|
||||
MethodOutcome outcome = myPatientDao.validate(p, null, raw, EncodingEnum.JSON, null, null, mySrd);
|
||||
@Test
|
||||
public void createStructDefThenValidatePatientWithRealUrl() throws IOException {
|
||||
// setup
|
||||
createStructureDefinitionInDao();
|
||||
|
||||
String encoded = myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(outcome.getOperationOutcome());
|
||||
ourLog.info("OO: {}", encoded);
|
||||
assertThat(encoded, containsString("No issues detected"));
|
||||
// execute
|
||||
final String outcomePatientValidate = validate(PATIENT_WITH_REAL_URL);
|
||||
|
||||
// verify
|
||||
assertExpectedOutcome(outcomePatientValidate);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void validatePatientWithFakeUrlStructDefThenValidatePatientWithRealUrl() throws IOException {
|
||||
// setup
|
||||
final String outcomePatientValidateFakeUrl = validate(PATIENT_WITH_FAKE_URL);
|
||||
assertTrue(outcomePatientValidateFakeUrl.contains(I18nConstants.VALIDATION_VAL_PROFILE_UNKNOWN_NOT_POLICY));
|
||||
createStructureDefinitionInDao();
|
||||
|
||||
// execute
|
||||
final String outcomePatientValidateRealUrl = validate(PATIENT_WITH_REAL_URL);
|
||||
|
||||
// verify
|
||||
assertExpectedOutcome(outcomePatientValidateRealUrl);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void validatePatientRealUrlThenCreateStructDefThenValidatePatientWithRealUrl() throws IOException {
|
||||
// setup
|
||||
final String outcomePatientValidateInitial = validate(PATIENT_WITH_REAL_URL);
|
||||
assertTrue(outcomePatientValidateInitial.contains(I18nConstants.VALIDATION_VAL_PROFILE_UNKNOWN_NOT_POLICY));
|
||||
createStructureDefinitionInDao();
|
||||
|
||||
// execute
|
||||
final String outcomePatientValidateAfterStructDef = validate(PATIENT_WITH_REAL_URL);
|
||||
|
||||
// verify
|
||||
assertExpectedOutcome(outcomePatientValidateAfterStructDef);
|
||||
}
|
||||
|
||||
private static void assertExpectedOutcome(String outcomeJson) {
|
||||
assertThat(outcomeJson, not(containsString(I18nConstants.VALIDATION_VAL_PROFILE_UNKNOWN_NOT_POLICY)));
|
||||
assertThat(outcomeJson, containsString("No issues detected"));
|
||||
}
|
||||
|
||||
private String validate(Patient thePatient) {
|
||||
final MethodOutcome validateOutcome = myPatientDao.validate(thePatient, null, myFhirContext.newJsonParser().encodeResourceToString(thePatient), EncodingEnum.JSON, null, null, mySrd);
|
||||
return myFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(validateOutcome.getOperationOutcome());
|
||||
}
|
||||
|
||||
private void createStructureDefinitionInDao() throws IOException {
|
||||
final StructureDefinition structureDefinition = loadResourceFromClasspath(StructureDefinition.class, "/r4/profile-differential-patient-r4.json");
|
||||
myStructureDefinitionDao.create(structureDefinition, new SystemRequestDetails());
|
||||
}
|
||||
|
||||
private static Patient createPatient(String theUrl) {
|
||||
final Patient patient = new Patient();
|
||||
patient.getText().setStatus(Narrative.NarrativeStatus.GENERATED);
|
||||
patient.getText().getDiv().setValue("<div>hello</div>");
|
||||
patient.getMeta().addProfile(theUrl);
|
||||
patient.setActive(true);
|
||||
return patient;
|
||||
}
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
|
|
|
@ -254,6 +254,13 @@ public class CachingValidationSupport extends BaseValidationSupportWrapper imple
|
|||
Optional<T> result = (Optional<T>) theCache.get(theKey, loaderWrapper);
|
||||
assert result != null;
|
||||
|
||||
// UGH! Animal sniffer :(
|
||||
if (!result.isPresent()) {
|
||||
ourLog.debug(
|
||||
"Invalidating cache entry for key: {} since the result of the underlying query is empty", theKey);
|
||||
theCache.invalidate(theKey);
|
||||
}
|
||||
|
||||
return result.orElse(null);
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue