From c37ed400989b93d0021d744c54b1e830302d6126 Mon Sep 17 00:00:00 2001 From: Martha Date: Fri, 22 Nov 2024 16:50:56 -0800 Subject: [PATCH 1/3] Increase canonical resource validation cache timeout. Update logic so that canonical CodeSystem resources are also cached. Use the terminology troubleshooting logs throughout the validation module. --- .../uhn/fhir/system/HapiSystemProperties.java | 28 ++- ...alidation-errors-profile-cache-expiry.yaml | 8 + .../VersionSpecificWorkerContextR4Test.java | 202 ++++++++++++++++ .../src/test/resources/logback-test.xml | 2 + .../bundle-with-medicationdispense.json | 102 ++++++++ .../profile-practitionerrole.json | 104 +++++++++ .../ca/uhn/fhir/jpa/test/BaseJpaTest.java | 3 +- .../validation/ValidatorResourceFetcher.java | 15 +- .../dstu3/hapi/ctx/HapiWorkerContext.java | 2 +- .../fhir/r4/hapi/ctx/HapiWorkerContext.java | 2 +- .../fhir/r4b/hapi/ctx/HapiWorkerContext.java | 2 +- .../fhir/r5/hapi/ctx/HapiWorkerContext.java | 2 +- .../validation/ValidationTestUtil.java | 32 +++ hapi-fhir-validation/pom.xml | 1 - .../BaseStaticResourceValidationSupport.java | 1 + .../support/BaseValidationSupport.java | 2 +- .../support/BaseValidationSupportWrapper.java | 2 +- .../support/CachingValidationSupport.java | 9 +- .../CommonCodeSystemsTerminologyService.java | 4 +- ...ltProfileValidationSupportNpmStrategy.java | 4 +- .../PrePopulatedValidationSupport.java | 3 +- ...teTerminologyServiceValidationSupport.java | 4 +- .../SnapshotGeneratingValidationSupport.java | 4 +- ...ownCodeSystemWarningValidationSupport.java | 4 +- .../support/ValidationSupportChain.java | 12 +- .../support/ValidationSupportUtils.java | 6 +- .../validator/FhirInstanceValidator.java | 19 +- .../validator/ValidatorWrapper.java | 4 +- .../VersionSpecificWorkerContextWrapper.java | 218 ++++++++++-------- ...rsionSpecificWorkerContextWrapperTest.java | 199 +++++++++------- 30 files changed, 765 insertions(+), 235 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6505-validation-errors-profile-cache-expiry.yaml create mode 100644 hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/validation/VersionSpecificWorkerContextR4Test.java create mode 100644 hapi-fhir-jpaserver-test-r4/src/test/resources/validation/practitionerrole/bundle-with-medicationdispense.json create mode 100644 hapi-fhir-jpaserver-test-r4/src/test/resources/validation/practitionerrole/profile-practitionerrole.json create mode 100644 hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/validation/ValidationTestUtil.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/system/HapiSystemProperties.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/system/HapiSystemProperties.java index 7d7c44761c71..4e6a568acc84 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/system/HapiSystemProperties.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/system/HapiSystemProperties.java @@ -19,7 +19,9 @@ */ package ca.uhn.fhir.system; -import org.apache.commons.lang3.time.DateUtils; +import com.google.common.annotations.VisibleForTesting; + +import java.util.concurrent.TimeUnit; public final class HapiSystemProperties { static final String SUPPRESS_HAPI_FHIR_VERSION_LOG = "suppress_hapi_fhir_version_log"; @@ -27,15 +29,14 @@ public final class HapiSystemProperties { /** * This is provided for testing only! Use with caution as this property may change. */ - static final String TEST_SYSTEM_PROP_VALIDATION_RESOURCE_CACHES_MS = - "TEST_SYSTEM_PROP_VALIDATION_RESOURCE_CACHES_MS"; + static final String VALIDATION_RESOURCE_CACHE_TIMEOUT_MILLIS = "VALIDATION_RESOURCE_CACHE_EXPIRY_MS"; static final String UNIT_TEST_CAPTURE_STACK = "unit_test_capture_stack"; static final String STACKFILTER_PATTERN_PROP = "log.stackfilter.pattern"; static final String HAPI_CLIENT_KEEPRESPONSES = "hapi.client.keepresponses"; static final String TEST_MODE = "test"; static final String UNIT_TEST_MODE = "unit_test_mode"; - static final long DEFAULT_TEST_SYSTEM_PROP_VALIDATION_RESOURCE_CACHES_MS = 10 * DateUtils.MILLIS_PER_SECOND; + static final long DEFAULT_VALIDATION_RESOURCE_CACHE_TIMEOUT_MILLIS = TimeUnit.MINUTES.toMillis(10); static final String PREVENT_INVALIDATING_CONDITIONAL_MATCH_CRITERIA = "hapi.storage.prevent_invalidating_conditional_match_criteria"; @@ -75,24 +76,31 @@ public static void setStackFilterPattern(String thePattern) { /** * Set the validation resource cache expireAfterWrite timeout in milliseconds * - * @param theMillis + * @param theMillis the timeout value to set (in milliseconds) */ - public static void setTestValidationResourceCachesMs(long theMillis) { - System.setProperty(TEST_SYSTEM_PROP_VALIDATION_RESOURCE_CACHES_MS, "" + theMillis); + public static void setValidationResourceCacheTimeoutMillis(long theMillis) { + System.setProperty(VALIDATION_RESOURCE_CACHE_TIMEOUT_MILLIS, "" + theMillis); } /** * Get the validation resource cache expireAfterWrite timeout in milliseconds. If it has not been set, the default * value is 10 seconds. */ - public static long getTestValidationResourceCachesMs() { - String property = System.getProperty(TEST_SYSTEM_PROP_VALIDATION_RESOURCE_CACHES_MS); + public static long getValidationResourceCacheTimeoutMillis() { + String property = System.getProperty(VALIDATION_RESOURCE_CACHE_TIMEOUT_MILLIS); if (property == null) { - return DEFAULT_TEST_SYSTEM_PROP_VALIDATION_RESOURCE_CACHES_MS; + return DEFAULT_VALIDATION_RESOURCE_CACHE_TIMEOUT_MILLIS; } return Long.parseLong(property); } + @VisibleForTesting + public static void restoreDefaultValidationResourceCacheTimeoutMillis() { + System.clearProperty(VALIDATION_RESOURCE_CACHE_TIMEOUT_MILLIS); + System.setProperty( + VALIDATION_RESOURCE_CACHE_TIMEOUT_MILLIS, "" + DEFAULT_VALIDATION_RESOURCE_CACHE_TIMEOUT_MILLIS); + } + /** * When this property is primarily used to control application shutdown behavior */ diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6505-validation-errors-profile-cache-expiry.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6505-validation-errors-profile-cache-expiry.yaml new file mode 100644 index 000000000000..baa8112ac118 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6505-validation-errors-profile-cache-expiry.yaml @@ -0,0 +1,8 @@ +--- +type: fix +issue: 6505 +title: "Previously, when invoking a $validate operation that took more than 10s some unexpected errors +would be returned which suggest the profile was not applied properly or used. +That happens because the validation cache entry expired within the $validate call +and the FHIR Core Validation library expects the same object instance for the same profile to be returned. +This has been fixed by increasing the cache expiry to 10min." diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/validation/VersionSpecificWorkerContextR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/validation/VersionSpecificWorkerContextR4Test.java new file mode 100644 index 000000000000..6789c0505907 --- /dev/null +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/validation/VersionSpecificWorkerContextR4Test.java @@ -0,0 +1,202 @@ +package ca.uhn.fhir.jpa.validation; + +import ca.uhn.fhir.context.support.IValidationSupport; +import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test; +import ca.uhn.fhir.system.HapiSystemProperties; +import ca.uhn.fhir.util.ClasspathUtil; +import org.apache.commons.lang3.StringUtils; +import org.hl7.fhir.common.hapi.validation.validator.FhirInstanceValidator; +import org.hl7.fhir.common.hapi.validation.validator.VersionSpecificWorkerContextWrapper; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.CodeSystem; +import org.hl7.fhir.r4.model.ElementDefinition; +import org.hl7.fhir.r4.model.StructureDefinition; +import org.hl7.fhir.r4.model.ValueSet; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import java.util.List; + +import static ca.uhn.fhir.test.utilities.validation.ValidationTestUtil.getValidationErrors; +import static org.assertj.core.api.Assertions.assertThat; + +public class VersionSpecificWorkerContextR4Test extends BaseResourceProviderR4Test { + @Autowired + private FhirInstanceValidator myInstanceValidator; + @Autowired + private IValidationSupport myValidationSupport; + + @Nested + public class ValidateWithCacheNegativeTest { + @BeforeEach + public void before() { + HapiSystemProperties.setValidationResourceCacheTimeoutMillis(1); + myInstanceValidator.resetWorkerContext(); + } + + /** + * If the cache is set to expire quickly, the second retrieval of the profile will result in a different StructureDefinition instance. + * The FHIR Core validation library currently expects the same instance of StructureDefinition to be returned for the same profile. + * This test can be removed if the FHIR Core library is updated to use equals/hashcode but for not this is how it works. + */ + @Test + public void validate_returnsUnexpectedErrors() { + // setup + StructureDefinition profileProcedure = ClasspathUtil.loadResource(myFhirContext, StructureDefinition.class, "validation/practitionerrole/profile-practitionerrole.json"); + myClient.update().resource(profileProcedure).execute(); + + Bundle bundle = ClasspathUtil.loadResource(myFhirContext, Bundle.class, "validation/practitionerrole/bundle-with-medicationdispense.json"); + + // execute and verify + // the cache entry for the StructureDefinition (profile) will expire at the timeout (1ms) after it is written + // there are multiple calls to fetch the profile within a single validate call, which means subsequent ones will get a different instance of StructureDefinition + List errors = getValidationErrors(myClient, bundle); + assertThat(StringUtils.join("", errors)).contains("PractitionerRole.telecom:TelecomPhone: max allowed = 1, but found 2"); + } + } + + @Nested + public class ValidateWithCachePositiveTest { + @BeforeEach + public void before() { + HapiSystemProperties.restoreDefaultValidationResourceCacheTimeoutMillis(); + myInstanceValidator.resetWorkerContext(); + } + + @Test + public void validate_profileResourceFetchedMultipleTimes_returnsNoErrors() { + // setup + StructureDefinition profileProcedure = ClasspathUtil.loadResource(myFhirContext, StructureDefinition.class, "validation/practitionerrole/profile-practitionerrole.json"); + myClient.update().resource(profileProcedure).execute(); + + Bundle bundle = ClasspathUtil.loadResource(myFhirContext, Bundle.class, "validation/practitionerrole/bundle-with-medicationdispense.json"); + + // execute and verify + List errors = getValidationErrors(myClient, bundle); + assertThat(errors).isEmpty(); + } + } + + @Nested + public class FetchTests { + private VersionSpecificWorkerContextWrapper myWorkerContext; + + @BeforeEach + public void before() { + myWorkerContext = VersionSpecificWorkerContextWrapper.newVersionSpecificWorkerContextWrapper(myValidationSupport); + } + + @Test + public void fetchResource_withStructureDefinition_usesCache() { + final String resourceUrl = createStructureDefinition(); + + // verify that the snapshot is generated + org.hl7.fhir.r5.model.StructureDefinition fetchedStructureDefinition1 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.StructureDefinition.class, resourceUrl); + assertThat(fetchedStructureDefinition1.hasSnapshot()).isTrue(); + + // verify that the sub-subsequent fetchResource returns the same resource instance from the cache + org.hl7.fhir.r5.model.StructureDefinition fetchedStructureDefinition2 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.StructureDefinition.class, resourceUrl); + assertThat(fetchedStructureDefinition2).isSameAs(fetchedStructureDefinition1); + } + + @Test + public void fetchResource_withCodeSystem_cacheIsUsed() { + final String resourceUrl = "http://example.com/CodeSystem/example-system"; + + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setId("CodeSystem/example-system"); + codeSystem.setUrl(resourceUrl); + + myCodeSystemDao.create(codeSystem, mySrd); + + org.hl7.fhir.r5.model.CodeSystem fetchedCodeSystem1 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.CodeSystem.class, resourceUrl); + + // verify that the sub-subsequent fetchResource returns the same resource instance from the cache + org.hl7.fhir.r5.model.CodeSystem fetchedCodeSystem2 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.CodeSystem.class, resourceUrl); + assertThat(fetchedCodeSystem2).isSameAs(fetchedCodeSystem1); + } + + @Test + public void fetchCodeSystem_cacheIsUsed() { + final String resourceUrl = "http://example.com/CodeSystem/example-system"; + + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setId("CodeSystem/example-system"); + codeSystem.setUrl(resourceUrl); + + myCodeSystemDao.create(codeSystem, mySrd); + + org.hl7.fhir.r5.model.CodeSystem fetchedCodeSystem1 = myWorkerContext.fetchCodeSystem(resourceUrl); + + // verify that the sub-subsequent fetchCodeSystem returns the same resource instance from the cache + org.hl7.fhir.r5.model.CodeSystem fetchedCodeSystem2 = myWorkerContext.fetchCodeSystem(resourceUrl); + assertThat(fetchedCodeSystem2).isSameAs(fetchedCodeSystem1); + } + + @Test + public void fetchCodeSystem_usesSameCacheAsFetchResource() { + final String resourceUrl = "http://example.com/CodeSystem/example-system"; + + CodeSystem codeSystem = new CodeSystem(); + codeSystem.setId("CodeSystem/example-system"); + codeSystem.setUrl(resourceUrl); + + myCodeSystemDao.create(codeSystem, mySrd); + + org.hl7.fhir.r5.model.CodeSystem fetchedCodeSystem1 = myWorkerContext.fetchCodeSystem(resourceUrl); + + // verify that the sub-subsequent fetchResource returns the same resource instance from the cache + org.hl7.fhir.r5.model.CodeSystem fetchedCodeSystem2 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.CodeSystem.class, resourceUrl); + assertThat(fetchedCodeSystem2).isSameAs(fetchedCodeSystem1); + } + + @Test + public void fetchResource_withValueSet_cacheIsUsed() { + final String resourceUrl = "http://example.com/ValueSet/example-set"; + + ValueSet valueSet = new ValueSet(); + valueSet.setId("ValueSet/example-set"); + valueSet.setUrl(resourceUrl); + + myValueSetDao.create(valueSet, mySrd); + + org.hl7.fhir.r5.model.ValueSet fetchedCodeSystem1 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.ValueSet.class, resourceUrl); + + // verify that the sub-subsequent fetchResource returns the same resource instance from the cache + org.hl7.fhir.r5.model.ValueSet fetchedCodeSystem2 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.ValueSet.class, resourceUrl); + assertThat(fetchedCodeSystem2).isSameAs(fetchedCodeSystem1); + } + + @Test + public void fetchResource_expireCache_cacheWorksAsExpected() { + final String resourceUrl = createStructureDefinition(); + + org.hl7.fhir.r5.model.StructureDefinition fetchedStructureDefinition1 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.StructureDefinition.class, resourceUrl); + + // simulate cache timeout by invalidating cache + myWorkerContext.invalidateCaches(); + + // verify that the sub-subsequent fetchResource returns a different resource instance from the cache + org.hl7.fhir.r5.model.StructureDefinition fetchedStructureDefinition3 = myWorkerContext.fetchResource(org.hl7.fhir.r5.model.StructureDefinition.class, resourceUrl); + assertThat(fetchedStructureDefinition3).isNotSameAs(fetchedStructureDefinition1); + } + } + + private String createStructureDefinition() { + final String resourceUrl = "http://example.com/StructureDefinition/example-profile-patient"; + StructureDefinition structureDefinition = new StructureDefinition(); + structureDefinition.setId("StructureDefinition/example-profile-patient"); + structureDefinition.setUrl(resourceUrl); + structureDefinition.setType("Patient"); + structureDefinition.setBaseDefinition("http://hl7.org/fhir/StructureDefinition/Patient"); + structureDefinition.setDerivation(StructureDefinition.TypeDerivationRule.CONSTRAINT); + ElementDefinition telecomElement = structureDefinition.getDifferential().addElement().setPath("Patient.telecom"); + telecomElement.setMax("1"); + + myStructureDefinitionDao.create(structureDefinition, mySrd); + + return resourceUrl; + } +} diff --git a/hapi-fhir-jpaserver-test-r4/src/test/resources/logback-test.xml b/hapi-fhir-jpaserver-test-r4/src/test/resources/logback-test.xml index dcada7fec76c..89eff81d8ac3 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/resources/logback-test.xml +++ b/hapi-fhir-jpaserver-test-r4/src/test/resources/logback-test.xml @@ -43,4 +43,6 @@ + + diff --git a/hapi-fhir-jpaserver-test-r4/src/test/resources/validation/practitionerrole/bundle-with-medicationdispense.json b/hapi-fhir-jpaserver-test-r4/src/test/resources/validation/practitionerrole/bundle-with-medicationdispense.json new file mode 100644 index 000000000000..6224421f086a --- /dev/null +++ b/hapi-fhir-jpaserver-test-r4/src/test/resources/validation/practitionerrole/bundle-with-medicationdispense.json @@ -0,0 +1,102 @@ +{ + "resourceType": "Bundle", + "type": "document", + "identifier": { + "system": "http://example.com/fhir/NamingSystem/id-pharmacy-med-document", + "value": "11111121" + }, + "timestamp": "2024-11-01T19:33:57-05:00", + "entry": [ + { + "fullUrl": "urn:uuid:5d7d0abe-578a-4a9c-90c0-1f8184911648", + "resource": { + "resourceType": "Composition", + "text": { + "status": "empty", + "div": "
empty
" + }, + "status": "final", + "type": { + "coding": [ + { + "system": "http://loinc.org", + "code": "29550-1", + "display": "Medication dispensed" + } + ] + }, + "date": "2023-10-10T12:30:02Z", + "author": [ + { + "reference": "urn:uuid:9fb30dc4-c210-418a-98cd-2ef0936081d4" + } + ], + "title": "Medication dispensed", + "section": [ + { + "title": "Medication dispensed", + "code": { + "coding": [ + { + "system": "http://loinc.org", + "code": "29550-1", + "display": "Medication dispensed" + } + ] + }, + "entry": [ + { + "reference": "urn:uuid:1e88351c-9018-4c48-b596-ed22ead5ec6a" + } + ] + } + ] + } + }, + { + "fullUrl": "urn:uuid:9fb30dc4-c210-418a-98cd-2ef0936081d4", + "resource": { + "resourceType": "PractitionerRole", + "meta": { + "profile": [ + "http://example.com/fhir/StructureDefinition/profile-PractitionerRole" + ] + }, + "text": { + "status": "empty", + "div": "
empty
" + } + } + }, + { + "fullUrl": "urn:uuid:1e88351c-9018-4c48-b596-ed22ead5ec6a", + "resource": { + "resourceType": "PractitionerRole", + "meta": { + "profile": [ + "http://example.com/fhir/StructureDefinition/profile-PractitionerRole" + ] + }, + "text": { + "status": "empty", + "div": "
empty
" + }, + "practitioner": { + "reference": "urn:uuid:a902ede0-414e-41a8-a99d-970ff5ee6ea9" + }, + "telecom": [ + { + "system": "phone", + "value": "905-555-5556", + "use": "work" + }, + { + "system": "fax", + "value": "416-555-5555", + "use": "work" + } + ] + } + } + ] +} \ No newline at end of file diff --git a/hapi-fhir-jpaserver-test-r4/src/test/resources/validation/practitionerrole/profile-practitionerrole.json b/hapi-fhir-jpaserver-test-r4/src/test/resources/validation/practitionerrole/profile-practitionerrole.json new file mode 100644 index 000000000000..a8023c4b66df --- /dev/null +++ b/hapi-fhir-jpaserver-test-r4/src/test/resources/validation/practitionerrole/profile-practitionerrole.json @@ -0,0 +1,104 @@ +{ + "resourceType": "StructureDefinition", + "text": { + "status": "empty" + }, + "id": "example-profile-PractitionerRole", + "url": "http://example.com/fhir/StructureDefinition/profile-PractitionerRole", + "version": "4.0.1", + "name": "PractitionerRole", + "title": "PractitionerRole (Submission)", + "status": "active", + "fhirVersion": "4.0.1", + "kind": "resource", + "abstract": false, + "type": "PractitionerRole", + "baseDefinition": "http://hl7.org/fhir/StructureDefinition/PractitionerRole", + "derivation": "constraint", + "differential": { + "element": [ + { + "id": "PractitionerRole", + "path": "PractitionerRole", + "mustSupport": true + }, + { + "id": "PractitionerRole.meta", + "path": "PractitionerRole.meta", + "min": 1, + "mustSupport": true + }, + { + "id": "PractitionerRole.meta.profile", + "path": "PractitionerRole.meta.profile", + "min": 1, + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom", + "path": "PractitionerRole.telecom", + "slicing": { + "discriminator": [ + { + "type": "value", + "path": "system" + } + ], + "rules": "open" + }, + "max": "2", + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom:TelecomPhone", + "path": "PractitionerRole.telecom", + "sliceName": "TelecomPhone", + "max": "1", + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom:TelecomPhone.system", + "path": "PractitionerRole.telecom.system", + "min": 1, + "fixedCode": "phone", + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom:TelecomPhone.value", + "path": "PractitionerRole.telecom.value", + "min": 1, + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom:TelecomPhone.use", + "path": "PractitionerRole.telecom.use", + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom:TelecomFax", + "path": "PractitionerRole.telecom", + "sliceName": "TelecomFax", + "max": "1", + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom:TelecomFax.system", + "path": "PractitionerRole.telecom.system", + "min": 1, + "fixedCode": "fax", + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom:TelecomFax.value", + "path": "PractitionerRole.telecom.value", + "min": 1, + "mustSupport": true + }, + { + "id": "PractitionerRole.telecom:TelecomFax.use", + "path": "PractitionerRole.telecom.use", + "mustSupport": true + } + ] + } +} \ No newline at end of file diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaTest.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaTest.java index dba57255f05f..767e363c67f9 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/test/BaseJpaTest.java @@ -187,10 +187,11 @@ public abstract class BaseJpaTest extends BaseTest { protected static final String CS_URL_4 = "http://example.com/my_code_system4"; protected static final String VS_URL = "http://example.com/my_value_set"; protected static final String VS_URL_2 = "http://example.com/my_value_set2"; + protected static final long VALIDATION_CACHE_TIMEOUT_MILLIS = 1000; private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseJpaTest.class); static { - HapiSystemProperties.setTestValidationResourceCachesMs(1000); + HapiSystemProperties.setValidationResourceCacheTimeoutMillis(VALIDATION_CACHE_TIMEOUT_MILLIS); HapiSystemProperties.enableTestMode(); HapiSystemProperties.enableUnitTestMode(); TestUtil.setShouldRandomizeTimezones(false); diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/validation/ValidatorResourceFetcher.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/validation/ValidatorResourceFetcher.java index 14813f8b3943..55ae8578248e 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/validation/ValidatorResourceFetcher.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/validation/ValidatorResourceFetcher.java @@ -43,12 +43,15 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.net.URISyntaxException; import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Set; +/** + * Please note that this bean is not currently used as part of the $validate operation. + * The FHIR Core validation library uses {@link VersionSpecificWorkerContextWrapper} to retrieve validation resources. + */ public class ValidatorResourceFetcher implements IValidatorResourceFetcher { private static final Logger ourLog = LoggerFactory.getLogger(ValidatorResourceFetcher.class); @@ -105,10 +108,9 @@ private IBaseResource fetchByUrl(String url, IFhirResourceDao dao, RequestDet } catch (InvalidRequestException e) { ourLog.info("Resource does not support 'url' or 'version' Search Parameters"); } - if (results != null && results.size() > 0) { - if (results.size() > 1) { - ourLog.warn( - String.format("Multiple results found for URL '%s', only the first will be considered.", url)); + if (results != null && !results.isEmpty()) { + if (results.size() > 1 && ourLog.isWarnEnabled()) { + ourLog.warn("Multiple results found for URL '{}', only the first will be considered.", url); } return results.get(0); } else { @@ -134,8 +136,7 @@ public IValidatorResourceFetcher setLocale(Locale locale) { } @Override - public CanonicalResource fetchCanonicalResource(IResourceValidator validator, Object appContext, String url) - throws URISyntaxException { + public CanonicalResource fetchCanonicalResource(IResourceValidator validator, Object appContext, String url) { return null; } diff --git a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/ctx/HapiWorkerContext.java b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/ctx/HapiWorkerContext.java index 7b6f58cf7b7f..fba78b4efb30 100644 --- a/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/ctx/HapiWorkerContext.java +++ b/hapi-fhir-structures-dstu3/src/main/java/org/hl7/fhir/dstu3/hapi/ctx/HapiWorkerContext.java @@ -60,7 +60,7 @@ public HapiWorkerContext(FhirContext theCtx, IValidationSupport theValidationSup myCtx = theCtx; myValidationSupport = theValidationSupport; - long timeoutMillis = HapiSystemProperties.getTestValidationResourceCachesMs(); + long timeoutMillis = HapiSystemProperties.getValidationResourceCacheTimeoutMillis(); myFetchedResourceCache = CacheFactory.build(timeoutMillis); // Set a default locale setValidationMessageLanguage(getLocale()); diff --git a/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/ctx/HapiWorkerContext.java b/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/ctx/HapiWorkerContext.java index 436338c84435..a0f6d85a5772 100644 --- a/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/ctx/HapiWorkerContext.java +++ b/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/ctx/HapiWorkerContext.java @@ -83,7 +83,7 @@ public HapiWorkerContext(FhirContext theCtx, IValidationSupport theValidationSup myCtx = theCtx; myValidationSupport = theValidationSupport; - long timeoutMillis = HapiSystemProperties.getTestValidationResourceCachesMs(); + long timeoutMillis = HapiSystemProperties.getValidationResourceCacheTimeoutMillis(); myFetchedResourceCache = CacheFactory.build(timeoutMillis); diff --git a/hapi-fhir-structures-r4b/src/main/java/org/hl7/fhir/r4b/hapi/ctx/HapiWorkerContext.java b/hapi-fhir-structures-r4b/src/main/java/org/hl7/fhir/r4b/hapi/ctx/HapiWorkerContext.java index be36650d2636..b01d1d9b45e0 100644 --- a/hapi-fhir-structures-r4b/src/main/java/org/hl7/fhir/r4b/hapi/ctx/HapiWorkerContext.java +++ b/hapi-fhir-structures-r4b/src/main/java/org/hl7/fhir/r4b/hapi/ctx/HapiWorkerContext.java @@ -65,7 +65,7 @@ public HapiWorkerContext(FhirContext theCtx, IValidationSupport theValidationSup myCtx = theCtx; myValidationSupport = theValidationSupport; - long timeoutMillis = HapiSystemProperties.getTestValidationResourceCachesMs(); + long timeoutMillis = HapiSystemProperties.getValidationResourceCacheTimeoutMillis(); myFetchedResourceCache = CacheFactory.build(timeoutMillis); diff --git a/hapi-fhir-structures-r5/src/main/java/org/hl7/fhir/r5/hapi/ctx/HapiWorkerContext.java b/hapi-fhir-structures-r5/src/main/java/org/hl7/fhir/r5/hapi/ctx/HapiWorkerContext.java index 411d4b275966..7fbe0a53c6fa 100644 --- a/hapi-fhir-structures-r5/src/main/java/org/hl7/fhir/r5/hapi/ctx/HapiWorkerContext.java +++ b/hapi-fhir-structures-r5/src/main/java/org/hl7/fhir/r5/hapi/ctx/HapiWorkerContext.java @@ -67,7 +67,7 @@ public HapiWorkerContext(FhirContext theCtx, IValidationSupport theValidationSup myCtx = theCtx; myValidationSupport = theValidationSupport; - long timeoutMillis = HapiSystemProperties.getTestValidationResourceCachesMs(); + long timeoutMillis = HapiSystemProperties.getValidationResourceCacheTimeoutMillis(); myFetchedResourceCache = CacheFactory.build(timeoutMillis); diff --git a/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/validation/ValidationTestUtil.java b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/validation/ValidationTestUtil.java new file mode 100644 index 000000000000..56a3235fea13 --- /dev/null +++ b/hapi-fhir-test-utilities/src/main/java/ca/uhn/fhir/test/utilities/validation/ValidationTestUtil.java @@ -0,0 +1,32 @@ +package ca.uhn.fhir.test.utilities.validation; + +import ca.uhn.fhir.rest.api.MethodOutcome; +import ca.uhn.fhir.rest.client.api.IGenericClient; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.OperationOutcome; + +import java.util.List; + +/** + * Utility class for resource validation. + */ +public class ValidationTestUtil { + private ValidationTestUtil() { + // utility + } + + /** + * Invokes validate operation against the provided FHIR client with the provided FHIR resource. + * @param theClient the client + * @param theResource the resource + * @return the errors returned by the validate call + */ + public static List getValidationErrors(IGenericClient theClient, IBaseResource theResource) { + MethodOutcome result = theClient.validate().resource(theResource).execute(); + OperationOutcome operationOutcome = (OperationOutcome) result.getOperationOutcome(); + return operationOutcome.getIssue().stream() + .filter(issue -> issue.getSeverity() == OperationOutcome.IssueSeverity.ERROR) + .map(OperationOutcome.OperationOutcomeIssueComponent::getDiagnostics) + .toList(); + } +} diff --git a/hapi-fhir-validation/pom.xml b/hapi-fhir-validation/pom.xml index a4067215f262..e8113b1c3ea0 100644 --- a/hapi-fhir-validation/pom.xml +++ b/hapi-fhir-validation/pom.xml @@ -248,7 +248,6 @@ com.google.guava guava - test ca.uhn.hapi.fhir diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseStaticResourceValidationSupport.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseStaticResourceValidationSupport.java index f12a6c6ae219..14aed3c33f23 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseStaticResourceValidationSupport.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseStaticResourceValidationSupport.java @@ -9,6 +9,7 @@ import java.util.List; import java.util.Map; +@Deprecated(since = "7.6") public abstract class BaseStaticResourceValidationSupport extends BaseValidationSupport implements IValidationSupport { /** diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupport.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupport.java index adc638150fbc..f88d257c4901 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupport.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupport.java @@ -10,7 +10,7 @@ public abstract class BaseValidationSupport implements IValidationSupport { /** * Constructor */ - public BaseValidationSupport(FhirContext theFhirContext) { + protected BaseValidationSupport(FhirContext theFhirContext) { Validate.notNull(theFhirContext, "theFhirContext must not be null"); myCtx = theFhirContext; } diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupportWrapper.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupportWrapper.java index fe474ea40cd2..0595fc278d3e 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupportWrapper.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/BaseValidationSupportWrapper.java @@ -29,7 +29,7 @@ public abstract class BaseValidationSupportWrapper extends BaseValidationSupport * @param theFhirContext The FhirContext object (must be initialized for the appropriate FHIR version) * @param theWrap The validation support object to wrap */ - public BaseValidationSupportWrapper(FhirContext theFhirContext, IValidationSupport theWrap) { + protected BaseValidationSupportWrapper(FhirContext theFhirContext, IValidationSupport theWrap) { super(theFhirContext); Validate.notNull(theWrap, "theWrap must not be null"); diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/CachingValidationSupport.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/CachingValidationSupport.java index 7edb7effd39a..5f3b96fdc346 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/CachingValidationSupport.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/CachingValidationSupport.java @@ -9,6 +9,7 @@ import ca.uhn.fhir.context.support.ValueSetExpansionOptions; import ca.uhn.fhir.sl.cache.Cache; import ca.uhn.fhir.sl.cache.CacheFactory; +import ca.uhn.fhir.util.Logs; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import org.apache.commons.lang3.concurrent.BasicThreadFactory; @@ -16,7 +17,6 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.Collections; import java.util.HashMap; @@ -36,7 +36,7 @@ @SuppressWarnings("unchecked") public class CachingValidationSupport extends BaseValidationSupportWrapper implements IValidationSupport { - private static final Logger ourLog = LoggerFactory.getLogger(CachingValidationSupport.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); public static final ValueSetExpansionOptions EMPTY_EXPANSION_OPTIONS = new ValueSetExpansionOptions(); private final Cache myCache; @@ -95,6 +95,11 @@ public CachingValidationSupport( myIsEnabledValidationForCodingsLogicalAnd = theIsEnabledValidationForCodingsLogicalAnd; } + @Override + public String getName() { + return getFhirContext().getVersion().getVersion() + " Caching Validation Support"; + } + @Override public List fetchAllConformanceResources() { String key = "fetchAllConformanceResources"; diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/CommonCodeSystemsTerminologyService.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/CommonCodeSystemsTerminologyService.java index 1b12e20ec62e..ab7bc9dc188d 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/CommonCodeSystemsTerminologyService.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/CommonCodeSystemsTerminologyService.java @@ -9,6 +9,7 @@ import ca.uhn.fhir.context.support.ValidationSupportContext; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.util.ClasspathUtil; +import ca.uhn.fhir.util.Logs; import ca.uhn.hapi.converters.canonical.VersionCanonicalizer; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -32,7 +33,6 @@ import org.hl7.fhir.r5.model.Resource; import org.hl7.fhir.r5.model.ValueSet.ConceptReferenceComponent; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; import java.io.InputStream; @@ -69,7 +69,7 @@ public class CommonCodeSystemsTerminologyService implements IValidationSupport { public static final String ALL_LANGUAGES_VALUESET_URL = "http://hl7.org/fhir/ValueSet/all-languages"; public static final String USPS_CODESYSTEM_URL = "https://www.usps.com/"; public static final String USPS_VALUESET_URL = "http://hl7.org/fhir/us/core/ValueSet/us-core-usps-state"; - private static final Logger ourLog = LoggerFactory.getLogger(CommonCodeSystemsTerminologyService.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); private static final Map USPS_CODES = Collections.unmodifiableMap(buildUspsCodes()); private static final Map ISO_4217_CODES = Collections.unmodifiableMap(buildIso4217Codes()); private static final Map ISO_3166_CODES = Collections.unmodifiableMap(buildIso3166Codes()); diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/DefaultProfileValidationSupportNpmStrategy.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/DefaultProfileValidationSupportNpmStrategy.java index 78c5ea21eb05..dff4d8d89f82 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/DefaultProfileValidationSupportNpmStrategy.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/DefaultProfileValidationSupportNpmStrategy.java @@ -4,16 +4,16 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.util.Logs; import ca.uhn.fhir.util.StopWatch; import jakarta.annotation.Nonnull; import org.apache.commons.lang3.Validate; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.IOException; public class DefaultProfileValidationSupportNpmStrategy extends NpmPackageValidationSupport { - private static final Logger ourLog = LoggerFactory.getLogger(DefaultProfileValidationSupportNpmStrategy.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); /** * Constructor diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/PrePopulatedValidationSupport.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/PrePopulatedValidationSupport.java index f6240be2cc5e..edeee4a799ba 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/PrePopulatedValidationSupport.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/PrePopulatedValidationSupport.java @@ -33,8 +33,7 @@ * This class is an implementation of {@link IValidationSupport} which may be pre-populated * with a collection of validation resources to be used by the validator. */ -public class PrePopulatedValidationSupport extends BaseStaticResourceValidationSupport - implements IValidationSupport, ILockable { +public class PrePopulatedValidationSupport extends BaseValidationSupport implements IValidationSupport, ILockable { private final Map myUrlToCodeSystems; private final Map myUrlToStructureDefinitions; diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/RemoteTerminologyServiceValidationSupport.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/RemoteTerminologyServiceValidationSupport.java index 370f8b423dd2..3456b494e84a 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/RemoteTerminologyServiceValidationSupport.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/RemoteTerminologyServiceValidationSupport.java @@ -15,6 +15,7 @@ import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.util.BundleUtil; +import ca.uhn.fhir.util.Logs; import ca.uhn.fhir.util.ParametersUtil; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; @@ -36,7 +37,6 @@ import org.hl7.fhir.r4.model.StringType; import org.hl7.fhir.r4.model.Type; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.Collection; @@ -57,7 +57,7 @@ * operation in order to validate codes. */ public class RemoteTerminologyServiceValidationSupport extends BaseValidationSupport implements IValidationSupport { - private static final Logger ourLog = LoggerFactory.getLogger(RemoteTerminologyServiceValidationSupport.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); public static final String ERROR_CODE_UNKNOWN_CODE_IN_CODE_SYSTEM = "unknownCodeInSystem"; public static final String ERROR_CODE_UNKNOWN_CODE_IN_VALUE_SET = "unknownCodeInValueSet"; diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/SnapshotGeneratingValidationSupport.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/SnapshotGeneratingValidationSupport.java index a4cf3642b0a1..20b45ffbbe7a 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/SnapshotGeneratingValidationSupport.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/SnapshotGeneratingValidationSupport.java @@ -8,6 +8,7 @@ import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException; +import ca.uhn.fhir.util.Logs; import ca.uhn.hapi.converters.canonical.VersionCanonicalizer; import org.apache.commons.lang3.Validate; import org.hl7.fhir.common.hapi.validation.validator.ProfileKnowledgeWorkerR5; @@ -18,7 +19,6 @@ import org.hl7.fhir.r5.context.IWorkerContext; import org.hl7.fhir.utilities.validation.ValidationMessage; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.ArrayList; @@ -36,7 +36,7 @@ * */ public class SnapshotGeneratingValidationSupport implements IValidationSupport { - private static final Logger ourLog = LoggerFactory.getLogger(SnapshotGeneratingValidationSupport.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); private final FhirContext myCtx; private final VersionCanonicalizer myVersionCanonicalizer; diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/UnknownCodeSystemWarningValidationSupport.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/UnknownCodeSystemWarningValidationSupport.java index 1898292c4512..ad40100d00ef 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/UnknownCodeSystemWarningValidationSupport.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/UnknownCodeSystemWarningValidationSupport.java @@ -4,12 +4,12 @@ import ca.uhn.fhir.context.support.ConceptValidationOptions; import ca.uhn.fhir.context.support.LookupCodeRequest; import ca.uhn.fhir.context.support.ValidationSupportContext; +import ca.uhn.fhir.util.Logs; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBaseResource; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * This validation support module may be placed at the end of a {@link ValidationSupportChain} @@ -20,7 +20,7 @@ * in order to specify that unknown code systems should be allowed. */ public class UnknownCodeSystemWarningValidationSupport extends BaseValidationSupport { - private static final Logger ourLog = LoggerFactory.getLogger(UnknownCodeSystemWarningValidationSupport.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); public static final IssueSeverity DEFAULT_SEVERITY = IssueSeverity.ERROR; diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java index 4eea50efc119..0360d743538e 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportChain.java @@ -30,7 +30,7 @@ public class ValidationSupportChain implements IValidationSupport { static Logger ourLog = Logs.getTerminologyTroubleshootingLog(); - private List myChain; + private final List myChain; /** * Constructor @@ -130,7 +130,7 @@ public IBaseResource generateSnapshot( @Override public FhirContext getFhirContext() { - if (myChain.size() == 0) { + if (myChain.isEmpty()) { return null; } return myChain.get(0).getFhirContext(); @@ -214,9 +214,7 @@ public boolean isRemoteTerminologyServiceConfigured() { Optional remoteTerminologyService = myChain.stream() .filter(RemoteTerminologyServiceValidationSupport.class::isInstance) .findFirst(); - if (remoteTerminologyService.isPresent()) { - return true; - } + return remoteTerminologyService.isPresent(); } return false; } @@ -235,12 +233,12 @@ public List fetchAllConformanceResources() { @Override public List fetchAllStructureDefinitions() { - return doFetchStructureDefinitions(t -> t.fetchAllStructureDefinitions()); + return doFetchStructureDefinitions(IValidationSupport::fetchAllStructureDefinitions); } @Override public List fetchAllNonBaseStructureDefinitions() { - return doFetchStructureDefinitions(t -> t.fetchAllNonBaseStructureDefinitions()); + return doFetchStructureDefinitions(IValidationSupport::fetchAllNonBaseStructureDefinitions); } private List doFetchStructureDefinitions( diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportUtils.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportUtils.java index 7321f33d8c8e..5f8e5bb7ed7c 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportUtils.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/support/ValidationSupportUtils.java @@ -1,13 +1,13 @@ package org.hl7.fhir.common.hapi.validation.support; +import ca.uhn.fhir.util.Logs; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.r4.model.ValueSet; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public final class ValidationSupportUtils { - private static final Logger ourLog = LoggerFactory.getLogger(ValidationSupportUtils.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); private ValidationSupportUtils() {} @@ -128,6 +128,6 @@ private static String extractCodeSystemForCodeR5(org.hl7.fhir.r5.model.ValueSet } private static void logCodeAndValueSet(String theCode, String theValueSet) { - ourLog.trace("CodeSystem couldn't be extracted for code: {} for ValueSet: {}", theCode, theValueSet); + ourLog.debug("CodeSystem couldn't be extracted for code: {} for ValueSet: {}", theCode, theValueSet); } } diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/FhirInstanceValidator.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/FhirInstanceValidator.java index 33e8844f951d..3cafbef9fbc8 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/FhirInstanceValidator.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/FhirInstanceValidator.java @@ -6,6 +6,7 @@ import ca.uhn.fhir.context.support.IValidationSupport; import ca.uhn.fhir.validation.IInstanceValidatorModule; import ca.uhn.fhir.validation.IValidationContext; +import com.google.common.annotations.VisibleForTesting; import jakarta.annotation.Nonnull; import org.apache.commons.lang3.Validate; import org.hl7.fhir.exceptions.FHIRException; @@ -33,7 +34,7 @@ public class FhirInstanceValidator extends BaseValidatorBridge implements IInsta private boolean noTerminologyChecks = false; private boolean noExtensibleWarnings = false; private boolean noBindingMsgSuppressed = false; - private volatile VersionSpecificWorkerContextWrapper myWrappedWorkerContext; + private VersionSpecificWorkerContextWrapper myWrappedWorkerContext; private boolean errorForUnknownProfiles = true; private boolean assumeValidRestReferences; @@ -150,6 +151,17 @@ public void setValidationSupport(IValidationSupport theValidationSupport) { myWrappedWorkerContext = null; } + /** + * Resets the {@link VersionSpecificWorkerContextWrapper} so it can be created again. + * This method is needed for (integration) test purposes and should not be used otherwise. + */ + @VisibleForTesting + public void resetWorkerContext() { + // this method had to be added because the current bean is autowired in the DAOs (BaseHapiFhirResourceDao) + // so the only way to force it to reinitialize as of now is by resetting it + myWrappedWorkerContext = null; + } + /** * If set to {@literal true} (default is true) extensions which are not known to the * validator (e.g. because they have not been explicitly declared in a profile) will @@ -251,6 +263,11 @@ protected VersionSpecificWorkerContextWrapper provideWorkerContext() { return wrappedWorkerContext; } + @VisibleForTesting + public VersionSpecificWorkerContextWrapper getWorkerContext() { + return myWrappedWorkerContext; + } + public IValidationPolicyAdvisor getValidatorPolicyAdvisor() { return validatorPolicyAdvisor; } diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/ValidatorWrapper.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/ValidatorWrapper.java index ac383cc3ca7e..3a5454dbc5cf 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/ValidatorWrapper.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/ValidatorWrapper.java @@ -3,6 +3,7 @@ import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.rest.api.EncodingEnum; +import ca.uhn.fhir.util.Logs; import ca.uhn.fhir.util.XmlUtil; import ca.uhn.fhir.validation.IValidationContext; import com.google.gson.Gson; @@ -25,7 +26,6 @@ import org.hl7.fhir.utilities.validation.ValidationMessage; import org.hl7.fhir.validation.instance.InstanceValidator; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.w3c.dom.NodeList; @@ -39,7 +39,7 @@ class ValidatorWrapper { - private static final Logger ourLog = LoggerFactory.getLogger(ValidatorWrapper.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); private BestPracticeWarningLevel myBestPracticeWarningLevel; private boolean myAnyExtensionsAllowed; private boolean myErrorForUnknownProfiles; diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java index f0f3f41e7f9b..8ca8dcc8c5f6 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java @@ -9,6 +9,7 @@ import ca.uhn.fhir.sl.cache.CacheFactory; import ca.uhn.fhir.sl.cache.LoadingCache; import ca.uhn.fhir.system.HapiSystemProperties; +import ca.uhn.fhir.util.Logs; import ca.uhn.hapi.converters.canonical.VersionCanonicalizer; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; @@ -16,6 +17,7 @@ import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; import org.fhir.ucum.UcumService; +import org.hl7.fhir.common.hapi.validation.support.CachingValidationSupport; import org.hl7.fhir.common.hapi.validation.support.ValidationSupportUtils; import org.hl7.fhir.exceptions.FHIRException; import org.hl7.fhir.exceptions.TerminologyServiceException; @@ -50,10 +52,7 @@ import org.hl7.fhir.utilities.validation.ValidationMessage; import org.hl7.fhir.utilities.validation.ValidationOptions; import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import java.io.FileNotFoundException; -import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -67,13 +66,24 @@ import static org.apache.commons.lang3.StringUtils.isBlank; import static org.apache.commons.lang3.StringUtils.isNotBlank; +/** + * Implements a FHIR context which is used to interact with the FHIR standard libraries + * by implementing {@link IWorkerContext}. + */ public class VersionSpecificWorkerContextWrapper extends I18nBase implements IWorkerContext { - private static final Logger ourLog = LoggerFactory.getLogger(VersionSpecificWorkerContextWrapper.class); + private static final Logger ourLog = Logs.getTerminologyTroubleshootingLog(); private final ValidationSupportContext myValidationSupportContext; private final VersionCanonicalizer myVersionCanonicalizer; + + /** + * This caches canonical validation resources (R5). + * The cache(s) within {@link ValidationSupportContext} use the current versioned resource. + * This cache uses the other cache(s) to populate, and also does the translation to canonical. + */ private final LoadingCache myFetchResourceCache; - private volatile List myAllStructures; - private volatile Set myAllPrimitiveTypes; + + private List myAllStructures; + private Set myAllPrimitiveTypes; private Parameters myExpansionProfile; public VersionSpecificWorkerContextWrapper( @@ -81,19 +91,31 @@ public VersionSpecificWorkerContextWrapper( myValidationSupportContext = theValidationSupportContext; myVersionCanonicalizer = theVersionCanonicalizer; - long timeoutMillis = HapiSystemProperties.getTestValidationResourceCachesMs(); + long timeoutMillis = HapiSystemProperties.getValidationResourceCacheTimeoutMillis(); + long fetchTimeoutMillis = + CachingValidationSupport.CacheTimeouts.defaultValues().getMiscMillis(); + if (timeoutMillis < fetchTimeoutMillis) { + ourLog.warn( + "The {} cache expires at {}ms which is sooner than the underlying cache at {}ms.", + getClass().getSimpleName(), + timeoutMillis, + fetchTimeoutMillis); + } myFetchResourceCache = CacheFactory.build(timeoutMillis, 10000, key -> { String fetchResourceName = key.getResourceName(); + String uri = key.getUri(); + + ourLog.trace("Adding resource type {} with uri {} to the cache", fetchResourceName, uri); + if (myValidationSupportContext - .getRootValidationSupport() - .getFhirContext() - .getVersion() - .getVersion() - == FhirVersionEnum.DSTU2) { - if ("CodeSystem".equals(fetchResourceName)) { - fetchResourceName = "ValueSet"; - } + .getRootValidationSupport() + .getFhirContext() + .getVersion() + .getVersion() + == FhirVersionEnum.DSTU2 + && "CodeSystem".equals(fetchResourceName)) { + fetchResourceName = "ValueSet"; } Class fetchResourceType; @@ -107,23 +129,22 @@ public VersionSpecificWorkerContextWrapper( .getImplementingClass(); } - IBaseResource fetched = myValidationSupportContext - .getRootValidationSupport() - .fetchResource(fetchResourceType, key.getUri()); + IBaseResource fetched = + myValidationSupportContext.getRootValidationSupport().fetchResource(fetchResourceType, uri); Resource canonical = myVersionCanonicalizer.resourceToValidatorCanonical(fetched); if (canonical instanceof StructureDefinition) { StructureDefinition canonicalSd = (StructureDefinition) canonical; - if (canonicalSd.getSnapshot().isEmpty()) { + if (!canonicalSd.hasSnapshot()) { ourLog.info("Generating snapshot for StructureDefinition: {}", canonicalSd.getUrl()); fetched = myValidationSupportContext .getRootValidationSupport() - .generateSnapshot(theValidationSupportContext, fetched, "", null, ""); + .generateSnapshot(myValidationSupportContext, fetched, "", null, ""); Validate.isTrue( fetched != null, "StructureDefinition %s has no snapshot, and no snapshot generator is configured", - key.getUri()); + uri); canonical = myVersionCanonicalizer.resourceToValidatorCanonical(fetched); } } @@ -134,6 +155,57 @@ public VersionSpecificWorkerContextWrapper( setValidationMessageLanguage(getLocale()); } + /** + * Fetch a validation resource by resource type name and URI. + * Please note that this method does not exist in the implementing interface {@link IWorkerContext}. + * @param theResourceType the resource type + * @param theUri the uri + * @return the resource + * @param the returned resource type class + */ + public T fetchResource(String theResourceType, String theUri) { + ResourceKey key = getResourceKey(theResourceType, theUri); + if (key == null) { + return null; + } + @SuppressWarnings("unchecked") + T resource = (T) myFetchResourceCache.get(key); + return resource; + } + + /** + * Check if a validation resource exists by resource type name and URI. + * Please note that this method does not exist in the implementing interface {@link IWorkerContext}. + * @param theResourceType the resource type + * @param theUri the uri + * @return true if the validation resource exists, false otherwise + */ + public boolean hasResource(String theResourceType, String theUri) { + return fetchResource(theResourceType, theUri) != null; + } + + private static ResourceKey getResourceKey(String theResourceName, String theUri) { + if (isBlank(theUri)) { + ourLog.debug("Encountered unexpected uri {} with resource type {}", theUri, theResourceName); + return null; + } + + // validation does not currently support (not implemented) multiple versions of the same validation resource + // as such, version is not ignored when doing the resource lookup + + String uri = theUri; + // handle profile version, if present + if (theUri.contains("|")) { + String[] parts = theUri.split("\\|"); + if (parts.length == 2) { + uri = parts[0]; + } else { + ourLog.warn("Unrecognized resource uri {} for resource type {}", theUri, theResourceName); + } + } + return new ResourceKey(theResourceName, uri); + } + @Override public Set getBinaryKeysAsSet() { throw new UnsupportedOperationException(Msg.code(2118)); @@ -155,8 +227,7 @@ public int loadFromPackage(NpmPackage pi, IContextResourceLoader loader) throws } @Override - public int loadFromPackage(NpmPackage pi, IContextResourceLoader loader, List types) - throws FileNotFoundException, IOException, FHIRException { + public int loadFromPackage(NpmPackage pi, IContextResourceLoader loader, List types) throws FHIRException { throw new UnsupportedOperationException(Msg.code(653)); } @@ -250,7 +321,7 @@ private List allStructures() { throw new InternalErrorException(Msg.code(659) + e); } } - myAllStructures = retVal; + myAllStructures = Collections.unmodifiableList(retVal); } return retVal; @@ -408,9 +479,9 @@ public ValueSetExpansionOutcome expandVS( Resource src, ElementDefinition.ElementDefinitionBindingComponent binding, boolean cacheOk, - boolean Hierarchical) { + boolean hierarchical) { ValueSet valueSet = fetchResource(ValueSet.class, binding.getValueSet(), src); - return expandVS(valueSet, cacheOk, Hierarchical); + return expandVS(valueSet, cacheOk, hierarchical); } @Override @@ -435,30 +506,14 @@ public void setLocale(Locale locale) { @Override public CodeSystem fetchCodeSystem(String system) { - IBaseResource fetched = - myValidationSupportContext.getRootValidationSupport().fetchCodeSystem(system); - if (fetched == null) { - return null; - } - try { - return myVersionCanonicalizer.codeSystemToValidatorCanonical(fetched); - } catch (FHIRException e) { - throw new InternalErrorException(Msg.code(665) + e); - } + return fetchResource(CodeSystem.class, system); } @Override - public CodeSystem fetchCodeSystem(String system, String verison) { - IBaseResource fetched = - myValidationSupportContext.getRootValidationSupport().fetchCodeSystem(system); - if (fetched == null) { - return null; - } - try { - return myVersionCanonicalizer.codeSystemToValidatorCanonical(fetched); - } catch (FHIRException e) { - throw new InternalErrorException(Msg.code(1992) + e); - } + public CodeSystem fetchCodeSystem(String system, String version) { + // validation does not currently support (not implemented) multiple versions of the same validation resource + // as such, version is not ignored when doing the resource lookup + return fetchCodeSystem(system); } @Override @@ -498,26 +553,7 @@ public T fetchResourceRaw(Class class_, String uri) { @Override public T fetchResource(Class class_, String theUri) { - if (isBlank(theUri)) { - return null; - } - - String uri = theUri; - // handle profile version, if present - if (theUri.contains("|")) { - String[] parts = theUri.split("\\|"); - if (parts.length == 2) { - uri = parts[0]; - } else { - ourLog.warn("Unrecognized profile uri: {}", theUri); - } - } - - ResourceKey key = new ResourceKey(class_.getSimpleName(), uri); - @SuppressWarnings("unchecked") - T retVal = (T) myFetchResourceCache.get(key); - - return retVal; + return fetchResource(class_.getSimpleName(), theUri); } @Override @@ -635,7 +671,7 @@ private Set allPrimitiveTypes() { .map(StructureDefinition::getName) .filter(Objects::nonNull) .collect(collectingAndThen(toSet(), Collections::unmodifiableSet)); - myAllPrimitiveTypes = retVal; + myAllPrimitiveTypes = Collections.unmodifiableSet(retVal); } return retVal; @@ -668,12 +704,7 @@ public String getVersion() { @Override public boolean hasResource(Class class_, String uri) { - if (isBlank(uri)) { - return false; - } - - ResourceKey key = new ResourceKey(class_.getSimpleName(), uri); - return myFetchResourceCache.get(key) != null; + return hasResource(class_.getSimpleName(), uri); } @Override @@ -712,15 +743,13 @@ public org.hl7.fhir.r5.context.ILoggingService getLogger() { } @Override - public void setLogger(org.hl7.fhir.r5.context.ILoggingService logger) { + public void setLogger(@Nonnull org.hl7.fhir.r5.context.ILoggingService logger) { throw new UnsupportedOperationException(Msg.code(687)); } @Override public boolean supportsSystem(String system) { - return myValidationSupportContext - .getRootValidationSupport() - .isCodeSystemSupported(myValidationSupportContext, system); + return hasResource(CodeSystem.class, system); } @Override @@ -832,6 +861,11 @@ private ValidationResult doValidation( String theSystem, String theCode, String theDisplay) { + ourLog.trace( + "Calling validateCode for ValueSet {} CodeSystem {} and code {}", + theValueSet != null ? theValueSet.toString() : null, + theSystem, + theCode); IValidationSupport.CodeValidationResult result; if (theValueSet != null) { result = validateCodeInValueSet(theValueSet, theValidationOptions, theSystem, theCode, theDisplay); @@ -906,39 +940,23 @@ public ValidationResult validateCode(ValidationOptions theOptions, CodeableConce if (retVal.isOk()) { validationResultsOk.add(retVal); } else { - for (OperationOutcome.OperationOutcomeIssueComponent issue : retVal.getIssues()) { - issues.add(issue); - } + issues.addAll(retVal.getIssues()); } } - if (code.getCoding().size() > 0) { + if (!code.getCoding().isEmpty()) { if (!myValidationSupportContext.isEnabledValidationForCodingsLogicalAnd()) { if (validationResultsOk.size() == code.getCoding().size()) { return validationResultsOk.get(0); } - } else { - if (validationResultsOk.size() > 0) { - return validationResultsOk.get(0); - } + } else if (!validationResultsOk.isEmpty()) { + return validationResultsOk.get(0); } } return new ValidationResult(ValidationMessage.IssueSeverity.ERROR, null, issues); } - private static OperationOutcome.OperationOutcomeIssueComponent getOperationOutcomeTxIssueComponent( - String message, OperationOutcome.IssueType issueCode, String txIssueTypeCode) { - OperationOutcome.OperationOutcomeIssueComponent issue = new OperationOutcome.OperationOutcomeIssueComponent() - .setSeverity(OperationOutcome.IssueSeverity.ERROR) - .setDiagnostics(message); - issue.getDetails().setText(message); - - issue.setCode(issueCode); - issue.getDetails().addCoding("http://hl7.org/fhir/tools/CodeSystem/tx-issue-type", txIssueTypeCode, null); - return issue; - } - public void invalidateCaches() { myFetchResourceCache.invalidateAll(); } @@ -946,7 +964,9 @@ public void invalidateCaches() { @Override public List fetchResourcesByType(Class theClass) { if (theClass.equals(StructureDefinition.class)) { - return (List) allStructures(); + @SuppressWarnings("unchecked") + List allStructures = (List) allStructures(); + return allStructures; } throw new UnsupportedOperationException(Msg.code(650) + "Unable to fetch resources of type: " + theClass); } diff --git a/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java b/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java index 57aae8d96f99..f4ac7b399717 100644 --- a/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java +++ b/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java @@ -1,144 +1,179 @@ package org.hl7.fhir.common.hapi.validation.validator; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.context.FhirVersionEnum; import ca.uhn.fhir.context.support.IValidationSupport; import ca.uhn.fhir.context.support.ValidationSupportContext; import ca.uhn.fhir.fhirpath.BaseValidationTestWithInlineMocks; -import ca.uhn.fhir.i18n.HapiLocalizer; +import ca.uhn.fhir.system.HapiSystemProperties; +import ca.uhn.fhir.util.Logs; import ca.uhn.hapi.converters.canonical.VersionCanonicalizer; - -import org.hl7.fhir.r5.model.Resource; -import org.hl7.fhir.r5.model.StructureDefinition; -import org.hl7.fhir.r5.model.StructureDefinition.StructureDefinitionKind; -import org.hl7.fhir.r5.model.ValueSet; +import ca.uhn.test.util.LogbackTestExtension; +import ch.qos.logback.classic.Level; +import org.hl7.fhir.common.hapi.validation.support.CachingValidationSupport; +import org.hl7.fhir.r4.model.StructureDefinition; import org.hl7.fhir.utilities.validation.ValidationOptions; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.mockito.quality.Strictness; +import org.junit.jupiter.api.extension.RegisterExtension; + +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.mockito.Mockito.withSettings; - -import java.util.List; public class VersionSpecificWorkerContextWrapperTest extends BaseValidationTestWithInlineMocks { - final byte[] EXPECTED_BINARY_CONTENT_1 = "dummyBinaryContent1".getBytes(); - final byte[] EXPECTED_BINARY_CONTENT_2 = "dummyBinaryContent2".getBytes(); - final String EXPECTED_BINARY_KEY_1 = "dummyBinaryKey1"; - final String EXPECTED_BINARY_KEY_2 = "dummyBinaryKey2"; - final String NON_EXISTENT_BINARY_KEY = "nonExistentBinaryKey"; + private static final byte[] EXPECTED_BINARY_CONTENT_1 = "dummyBinaryContent1".getBytes(); + private static final byte[] EXPECTED_BINARY_CONTENT_2 = "dummyBinaryContent2".getBytes(); + private static final String EXPECTED_BINARY_KEY_1 = "dummyBinaryKey1"; + private static final String EXPECTED_BINARY_KEY_2 = "dummyBinaryKey2"; + private static final String NON_EXISTENT_BINARY_KEY = "nonExistentBinaryKey"; + + private static final FhirContext ourCtx = FhirContext.forR4Cached(); + private IValidationSupport myValidationSupport; + private ValidationSupportContext myValidationSupportContext; + private VersionSpecificWorkerContextWrapper myWorkerContextWrapper; + + public void setupValidationForBinary() { + myValidationSupport = mockValidationSupportWithTwoBinaries(); + myValidationSupportContext = mockValidationSupportContext(myValidationSupport); + VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(ourCtx); + myWorkerContextWrapper = new VersionSpecificWorkerContextWrapper(myValidationSupportContext, versionCanonicalizer); + } + + public void setupValidation() { + myValidationSupport = mockValidationSupport(); + myValidationSupportContext = mockValidationSupportContext(myValidationSupport); + VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(ourCtx); + myWorkerContextWrapper = new VersionSpecificWorkerContextWrapper(myValidationSupportContext, versionCanonicalizer); + } @Test public void hasBinaryKey_normally_returnsExpected() { - - IValidationSupport validationSupport = mockValidationSupportWithTwoBinaries(); - - ValidationSupportContext mockContext = mockValidationSupportContext(validationSupport); - - VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(FhirContext.forR5Cached()); - VersionSpecificWorkerContextWrapper wrapper = new VersionSpecificWorkerContextWrapper(mockContext, versionCanonicalizer); - - assertThat(wrapper.hasBinaryKey(EXPECTED_BINARY_KEY_1)).as("wrapper should have binary key " + EXPECTED_BINARY_KEY_1).isTrue(); - assertThat(wrapper.hasBinaryKey(EXPECTED_BINARY_KEY_2)).as("wrapper should have binary key " + EXPECTED_BINARY_KEY_1).isTrue(); - assertThat(wrapper.hasBinaryKey(NON_EXISTENT_BINARY_KEY)).as("wrapper should not have binary key " + NON_EXISTENT_BINARY_KEY).isFalse(); + setupValidationForBinary(); + assertThat(myWorkerContextWrapper.hasBinaryKey(EXPECTED_BINARY_KEY_1)).as("wrapper should have binary key " + EXPECTED_BINARY_KEY_1).isTrue(); + assertThat(myWorkerContextWrapper.hasBinaryKey(EXPECTED_BINARY_KEY_2)).as("wrapper should have binary key " + EXPECTED_BINARY_KEY_1).isTrue(); + assertThat(myWorkerContextWrapper.hasBinaryKey(NON_EXISTENT_BINARY_KEY)).as("wrapper should not have binary key " + NON_EXISTENT_BINARY_KEY).isFalse(); } @Test public void getBinaryForKey_normally_returnsExpected() { - - IValidationSupport validationSupport = mockValidationSupportWithTwoBinaries(); - - ValidationSupportContext mockContext = mockValidationSupportContext(validationSupport); - - VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(FhirContext.forR5Cached()); - VersionSpecificWorkerContextWrapper wrapper = new VersionSpecificWorkerContextWrapper(mockContext, versionCanonicalizer); - - assertThat(wrapper.getBinaryForKey(EXPECTED_BINARY_KEY_1)).containsExactly(EXPECTED_BINARY_CONTENT_1); - assertThat(wrapper.getBinaryForKey(EXPECTED_BINARY_KEY_2)).containsExactly(EXPECTED_BINARY_CONTENT_2); - assertThat(wrapper.getBinaryForKey(NON_EXISTENT_BINARY_KEY)).as("wrapper should return null for binary key " + NON_EXISTENT_BINARY_KEY).isNull(); + setupValidationForBinary(); + assertThat(myWorkerContextWrapper.getBinaryForKey(EXPECTED_BINARY_KEY_1)).containsExactly(EXPECTED_BINARY_CONTENT_1); + assertThat(myWorkerContextWrapper.getBinaryForKey(EXPECTED_BINARY_KEY_2)).containsExactly(EXPECTED_BINARY_CONTENT_2); + assertThat(myWorkerContextWrapper.getBinaryForKey(NON_EXISTENT_BINARY_KEY)).as("wrapper should return null for binary key " + NON_EXISTENT_BINARY_KEY).isNull(); } @Test - public void cacheResource_normally_executesWithoutException() { - - IValidationSupport validationSupport = mockValidationSupport(); + public void validateCode_codeInValueSet_resolvesCodeSystemFromValueSet() { + // setup + setupValidation(); - ValidationSupportContext mockContext = mockValidationSupportContext(validationSupport); + org.hl7.fhir.r5.model.ValueSet valueSet = new org.hl7.fhir.r5.model.ValueSet(); + valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system").addConcept().setCode("code0"); + valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system2").addConcept().setCode("code2"); + when(myValidationSupport.validateCodeInValueSet(any(), any(), any(), any(), any(), any())).thenReturn(mock(IValidationSupport.CodeValidationResult.class)); - VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(FhirContext.forR5Cached()); - VersionSpecificWorkerContextWrapper wrapper = new VersionSpecificWorkerContextWrapper(mockContext, versionCanonicalizer); + // execute + myWorkerContextWrapper.validateCode(new ValidationOptions(), "code0", valueSet); - wrapper.cacheResource(mock(Resource.class)); + // verify + verify(myValidationSupport, times(1)).validateCodeInValueSet(any(), any(), eq("http://codesystems.com/system"), eq("code0"), any(), any()); + verify(myValidationSupport, times(1)).validateCode(any(), any(), eq("http://codesystems.com/system"), eq("code0"), any(), any()); } @Test - public void validateCode_normally_resolvesCodeSystemFromValueSet() { - // setup - IValidationSupport validationSupport = mockValidationSupport(); - ValidationSupportContext mockContext = mockValidationSupportContext(validationSupport); - VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(FhirContext.forR5Cached()); - VersionSpecificWorkerContextWrapper wrapper = new VersionSpecificWorkerContextWrapper(mockContext, versionCanonicalizer); - - ValueSet valueSet = new ValueSet(); + public void validateCode_codeNotInValueSet_doesNotResolveSystem() { + setupValidation(); + org.hl7.fhir.r5.model.ValueSet valueSet = new org.hl7.fhir.r5.model.ValueSet(); valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system").addConcept().setCode("code0"); valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system2").addConcept().setCode("code2"); - when(validationSupport.fetchResource(eq(ValueSet.class), eq("http://somevalueset"))).thenReturn(valueSet); - when(validationSupport.validateCodeInValueSet(any(), any(), any(), any(), any(), any())).thenReturn(new IValidationSupport.CodeValidationResult()); // execute - wrapper.validateCode(new ValidationOptions(), "code0", valueSet); + myWorkerContextWrapper.validateCode(new ValidationOptions(), "code1", valueSet); // verify - verify(validationSupport, times(1)).validateCodeInValueSet(any(), any(), eq("http://codesystems.com/system"), eq("code0"), any(), any()); - verify(validationSupport, times(1)).validateCode(any(), any(), eq("http://codesystems.com/system"), eq("code0"), any(), any()); + verify(myValidationSupport, times(1)).validateCodeInValueSet(any(), any(), eq(null), eq("code1"), any(), any()); + verify(myValidationSupport, never()).validateCode(any(), any(), any(), any(), any(), any()); } @Test public void isPrimitive_primitive() { // setup - IValidationSupport validationSupport = mockValidationSupport(); - ValidationSupportContext mockContext = mockValidationSupportContext(validationSupport); - VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(FhirContext.forR5Cached()); - VersionSpecificWorkerContextWrapper wrapper = new VersionSpecificWorkerContextWrapper(mockContext, versionCanonicalizer); + setupValidation(); - List structDefs = createStructureDefinitions(); + List structDefs = createPrimitiveStructureDefinitions(); - when(mockContext.getRootValidationSupport().fetchAllStructureDefinitions()).thenReturn(structDefs); - assertThat(wrapper.isPrimitiveType("boolean")).isTrue(); + when(myValidationSupportContext.getRootValidationSupport().fetchAllStructureDefinitions()).thenReturn(structDefs); + assertThat(myWorkerContextWrapper.isPrimitiveType("boolean")).isTrue(); // try again to check if lookup after cache is built is working - assertThat(wrapper.isPrimitiveType("string")).isTrue(); + assertThat(myWorkerContextWrapper.isPrimitiveType("string")).isTrue(); } @Test public void isPrimitive_not_primitive() { // setup - IValidationSupport validationSupport = mockValidationSupport(); - ValidationSupportContext mockContext = mockValidationSupportContext(validationSupport); - VersionCanonicalizer versionCanonicalizer = new VersionCanonicalizer(FhirContext.forR5Cached()); - VersionSpecificWorkerContextWrapper wrapper = new VersionSpecificWorkerContextWrapper(mockContext, versionCanonicalizer); + setupValidation(); - List structDefs = createStructureDefinitions(); + List structDefs = createPrimitiveStructureDefinitions(); - when(mockContext.getRootValidationSupport().fetchAllStructureDefinitions()).thenReturn(structDefs); - assertThat(wrapper.isPrimitiveType("Person")).isFalse(); + when(myValidationSupportContext.getRootValidationSupport().fetchAllStructureDefinitions()).thenReturn(structDefs); + assertThat(myWorkerContextWrapper.isPrimitiveType("Person")).isFalse(); // try again to check if lookup after cache is built is working - assertThat(wrapper.isPrimitiveType("Organization")).isFalse(); + assertThat(myWorkerContextWrapper.isPrimitiveType("Organization")).isFalse(); // Assert that unknown types are not regarded as primitive - assertThat(wrapper.isPrimitiveType("Unknown")).isFalse(); + assertThat(myWorkerContextWrapper.isPrimitiveType("Unknown")).isFalse(); + } + + @Nested + public class ResourceCacheTimeoutTest { + @RegisterExtension + public final LogbackTestExtension myLogbackTestExtension = new LogbackTestExtension(Logs.getTerminologyTroubleshootingLog()); + + @Test + public void cacheTimeout_lessThanUnderlyingCacheTimeout_logsWarning() { + // setup + long persistenceTimeout = CachingValidationSupport.CacheTimeouts.defaultValues().getMiscMillis(); + HapiSystemProperties.setValidationResourceCacheTimeoutMillis(persistenceTimeout - 1); + long timeout = HapiSystemProperties.getValidationResourceCacheTimeoutMillis(); + + // test + setupValidation(); + + // verify + assertThat(timeout).isLessThan(persistenceTimeout); + String message = String.format("The VersionSpecificWorkerContextWrapper cache expires at %sms which is sooner than the underlying cache at %sms.", timeout, persistenceTimeout); + assertThat(myLogbackTestExtension.getLogEvents().stream().filter(event -> event.getLevel() == Level.WARN && event.getFormattedMessage().equals(message))).isNotEmpty(); + } + + @Test + public void cacheTimeout_valid_logsNoWarning() { + // setup + long persistenceTimeout = CachingValidationSupport.CacheTimeouts.defaultValues().getMiscMillis(); + HapiSystemProperties.restoreDefaultValidationResourceCacheTimeoutMillis(); + long timeout = HapiSystemProperties.getValidationResourceCacheTimeoutMillis(); + + // test + setupValidation(); + + // verify + assertThat(persistenceTimeout).isEqualTo(timeout); + String message = String.format("The VersionSpecificWorkerContextWrapper cache expires at %sms which is sooner than the underlying cache at %sms.", timeout, persistenceTimeout); + assertThat(myLogbackTestExtension.getLogEvents().stream().filter(event -> event.getLevel() == Level.WARN && event.getFormattedMessage().equals(message))).isEmpty(); + } } - private List createStructureDefinitions() { + private List createPrimitiveStructureDefinitions() { StructureDefinition stringType = createPrimitive("string"); StructureDefinition boolType = createPrimitive("boolean"); StructureDefinition personType = createComplex("Person"); @@ -148,11 +183,11 @@ private List createStructureDefinitions() { } private StructureDefinition createComplex(String name){ - return createStructureDefinition(name).setKind(StructureDefinitionKind.COMPLEXTYPE); + return createStructureDefinition(name).setKind(StructureDefinition.StructureDefinitionKind.COMPLEXTYPE); } private StructureDefinition createPrimitive(String name){ - return createStructureDefinition(name).setKind(StructureDefinitionKind.PRIMITIVETYPE); + return createStructureDefinition(name).setKind(StructureDefinition.StructureDefinitionKind.PRIMITIVETYPE); } private StructureDefinition createStructureDefinition(String name) { @@ -179,12 +214,8 @@ private static ValidationSupportContext mockValidationSupportContext(IValidation private static IValidationSupport mockValidationSupport() { - IValidationSupport mockValidationSupport; - mockValidationSupport = mock(IValidationSupport.class); - FhirContext mockFhirContext = mock(FhirContext.class, withSettings().strictness(Strictness.LENIENT)); - when(mockFhirContext.getLocalizer()).thenReturn(new HapiLocalizer()); - when(mockFhirContext.getVersion()).thenReturn(FhirVersionEnum.R4.getVersionImplementation()); - when(mockValidationSupport.getFhirContext()).thenReturn(mockFhirContext); + IValidationSupport mockValidationSupport = mock(IValidationSupport.class); + when(mockValidationSupport.getFhirContext()).thenReturn(ourCtx); return mockValidationSupport; } } From 2091291149df9b596f92fc28562e6de614fd297e Mon Sep 17 00:00:00 2001 From: Martha Date: Fri, 22 Nov 2024 18:10:51 -0800 Subject: [PATCH 2/3] Revert change to supportsSystem method and fix test. --- .../src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java | 5 +++++ .../uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java | 2 +- .../validator/VersionSpecificWorkerContextWrapper.java | 4 +++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java index c00ce3b74af1..97ed6baed864 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/term/api/ITermReadSvc.java @@ -56,6 +56,11 @@ */ public interface ITermReadSvc extends IValidationSupport { + @Override + default String getName() { + return getClass().getSimpleName() + " JPA Term Read Service"; + } + ValueSet expandValueSet( @Nullable ValueSetExpansionOptions theExpansionOptions, @Nonnull String theValueSetCanonicalUrl); diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java index 7cd70ab49e6a..ec893b704d23 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4ValidateTest.java @@ -1530,7 +1530,7 @@ public void validateResource_withUnknownMetaProfileurl_validatesButLogsWarning() // we have errors assertFalse(errors.isEmpty()); - LogbackTestExtensionAssert.assertThat(myLogbackTestExtension).hasWarnMessage("Unrecognized profile uri"); + LogbackTestExtensionAssert.assertThat(myLogbackTestExtension).hasWarnMessage("Unrecognized resource uri"); } @Test diff --git a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java index 8ca8dcc8c5f6..57a18966fe24 100644 --- a/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java +++ b/hapi-fhir-validation/src/main/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapper.java @@ -749,7 +749,9 @@ public void setLogger(@Nonnull org.hl7.fhir.r5.context.ILoggingService logger) { @Override public boolean supportsSystem(String system) { - return hasResource(CodeSystem.class, system); + return myValidationSupportContext + .getRootValidationSupport() + .isCodeSystemSupported(myValidationSupportContext, system); } @Override From d4fdee77343ef2f78f8713d912aac1081459309d Mon Sep 17 00:00:00 2001 From: Martha Date: Mon, 25 Nov 2024 13:52:22 -0800 Subject: [PATCH 3/3] Small change in test --- .../VersionSpecificWorkerContextWrapperTest.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java b/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java index f4ac7b399717..bc881fcb5a56 100644 --- a/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java +++ b/hapi-fhir-validation/src/test/java/org/hl7/fhir/common/hapi/validation/validator/VersionSpecificWorkerContextWrapperTest.java @@ -11,6 +11,7 @@ import ch.qos.logback.classic.Level; import org.hl7.fhir.common.hapi.validation.support.CachingValidationSupport; import org.hl7.fhir.r4.model.StructureDefinition; +import org.hl7.fhir.r5.model.ValueSet; import org.hl7.fhir.utilities.validation.ValidationOptions; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -55,7 +56,7 @@ public void setupValidation() { } @Test - public void hasBinaryKey_normally_returnsExpected() { + public void hasBinaryKey_returnsExpected() { setupValidationForBinary(); assertThat(myWorkerContextWrapper.hasBinaryKey(EXPECTED_BINARY_KEY_1)).as("wrapper should have binary key " + EXPECTED_BINARY_KEY_1).isTrue(); assertThat(myWorkerContextWrapper.hasBinaryKey(EXPECTED_BINARY_KEY_2)).as("wrapper should have binary key " + EXPECTED_BINARY_KEY_1).isTrue(); @@ -64,7 +65,7 @@ public void hasBinaryKey_normally_returnsExpected() { } @Test - public void getBinaryForKey_normally_returnsExpected() { + public void getBinaryForKey_returnsExpected() { setupValidationForBinary(); assertThat(myWorkerContextWrapper.getBinaryForKey(EXPECTED_BINARY_KEY_1)).containsExactly(EXPECTED_BINARY_CONTENT_1); assertThat(myWorkerContextWrapper.getBinaryForKey(EXPECTED_BINARY_KEY_2)).containsExactly(EXPECTED_BINARY_CONTENT_2); @@ -76,10 +77,12 @@ public void validateCode_codeInValueSet_resolvesCodeSystemFromValueSet() { // setup setupValidation(); - org.hl7.fhir.r5.model.ValueSet valueSet = new org.hl7.fhir.r5.model.ValueSet(); + ValueSet valueSet = new ValueSet(); valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system").addConcept().setCode("code0"); valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system2").addConcept().setCode("code2"); when(myValidationSupport.validateCodeInValueSet(any(), any(), any(), any(), any(), any())).thenReturn(mock(IValidationSupport.CodeValidationResult.class)); + when(myValidationSupport.fetchResource(ValueSet.class, eq("http://somevalueset"))).thenReturn(valueSet); + when(myValidationSupport.validateCodeInValueSet(any(), any(), any(), any(), any(), any())).thenReturn(new IValidationSupport.CodeValidationResult()); // execute myWorkerContextWrapper.validateCode(new ValidationOptions(), "code0", valueSet); @@ -92,7 +95,7 @@ public void validateCode_codeInValueSet_resolvesCodeSystemFromValueSet() { @Test public void validateCode_codeNotInValueSet_doesNotResolveSystem() { setupValidation(); - org.hl7.fhir.r5.model.ValueSet valueSet = new org.hl7.fhir.r5.model.ValueSet(); + ValueSet valueSet = new ValueSet(); valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system").addConcept().setCode("code0"); valueSet.getCompose().addInclude().setSystem("http://codesystems.com/system2").addConcept().setCode("code2");