From 92f09da809df6560586fc38af9cb35664995b3ef Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 1 Apr 2024 14:36:04 -0700 Subject: [PATCH 1/6] Updated fhirpath to point to receiving facility id --- etor/src/main/resources/rule_definitions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etor/src/main/resources/rule_definitions.json b/etor/src/main/resources/rule_definitions.json index 93c2ebb3e..ec90c3785 100644 --- a/etor/src/main/resources/rule_definitions.json +++ b/etor/src/main/resources/rule_definitions.json @@ -5,7 +5,7 @@ "violationMessage": "Message doesn't have required receiver id", "conditions": [ ], "validations": [ - "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.value.exists()" + "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.where(extension.valueString = 'HD.2,HD.3').value.exists()" ] }, { From 1d1f573c8eba1116d0b5029cc2584ca93e388b50 Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Mon, 1 Apr 2024 17:42:44 -0400 Subject: [PATCH 2/6] Add thread safety test and flag for rule loading --- .../etor/ruleengine/RuleEngine.java | 2 +- .../etor/ruleengine/RuleEngineTest.groovy | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java index cdf46ce84..756add0e6 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java @@ -28,7 +28,7 @@ public void unloadRules() { rules.clear(); } - public void ensureRulesLoaded() { + public synchronized void ensureRulesLoaded() { if (rules.isEmpty()) { loadRules(); } diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy index be678b9d7..50c9a16aa 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy @@ -39,6 +39,27 @@ class RuleEngineTest extends Specification { 1 * mockRuleLoader.loadRules(_ as String) >> [Mock(Rule)] } + def "ensureRulesLoaded loads rules only once on multiple threads"() { + given: + def threadsNum = 2 + def iterations = 4 + + when: + List threads = [] + (1..threadsNum).each { threadId -> + threads.add(new Thread({ + for (int i = 0; i < iterations; i++) { + ruleEngine.ensureRulesLoaded() + } + })) + } + threads*.start() + threads*.join() + + then: + 1 * mockRuleLoader.loadRules(_ as String) >> [Mock(Rule)] + } + def "ensureRulesLoaded logs an error if there is an exception loading the rules"() { given: def exception = new RuleLoaderException("Error loading rules", new Exception()) From 14e3edda61c3007110a54d62966a2724f6ec3d99 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 1 Apr 2024 14:54:24 -0700 Subject: [PATCH 3/6] Updated fhirpath --- etor/src/main/resources/rule_definitions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etor/src/main/resources/rule_definitions.json b/etor/src/main/resources/rule_definitions.json index ec90c3785..aa828918f 100644 --- a/etor/src/main/resources/rule_definitions.json +++ b/etor/src/main/resources/rule_definitions.json @@ -5,7 +5,7 @@ "violationMessage": "Message doesn't have required receiver id", "conditions": [ ], "validations": [ - "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.where(extension.valueString = 'HD.2,HD.3').value.exists()" + "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.where(system = 'urn:ietf:rfc:3986').value.exists()" ] }, { From 802ca6393bd543edee6f93a44163ce603037d899 Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Tue, 2 Apr 2024 14:04:50 -0400 Subject: [PATCH 4/6] Update RuleEngine.java Double checked locking added on ensureRulesLoaded --- .../trustedintermediary/etor/ruleengine/RuleEngine.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java index 756add0e6..0b72ad3eb 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngine.java @@ -28,9 +28,13 @@ public void unloadRules() { rules.clear(); } - public synchronized void ensureRulesLoaded() { + public void ensureRulesLoaded() { if (rules.isEmpty()) { - loadRules(); + synchronized (this) { + if (rules.isEmpty()) { + loadRules(); + } + } } } From 79c06f01d4837cceab44d2a25e4973ecfd4ffa90 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Tue, 2 Apr 2024 14:00:40 -0700 Subject: [PATCH 5/6] Updated receiver id fhirpath in tests and added new test to make sure we validate using the receiver id --- .../RuleEngineIntegrationTest.groovy | 76 ++++++++++++++++++- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineIntegrationTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineIntegrationTest.groovy index 1443cd964..7f698c876 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineIntegrationTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineIntegrationTest.groovy @@ -8,6 +8,10 @@ import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir import gov.hhs.cdc.trustedintermediary.wrappers.Logger import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter import org.hl7.fhir.r4.model.Bundle +import org.hl7.fhir.r4.model.Coding +import org.hl7.fhir.r4.model.MessageHeader +import org.hl7.fhir.r4.model.Organization +import org.hl7.fhir.r4.model.Reference import spock.lang.Specification import java.nio.file.Files @@ -80,7 +84,7 @@ class RuleEngineIntegrationTest extends Specification { where: testFile | validation "Orders/001_OML_O21_short.fhir" | "Bundle.entry.resource.ofType(MessageHeader).focus.resolve().category.exists()" - "Orders/003_AL_ORM_O01_NBS_Fully_Populated_1_hl7_translation.fhir" | "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.value.exists()" + "Orders/003_AL_ORM_O01_NBS_Fully_Populated_1_hl7_translation.fhir" | "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.where(system = 'urn:ietf:rfc:3986').value.exists()" // Once we fix the mapping for ORM from story #900 and update the FHIR files in /examples/Test/Orders, we can uncomment the below line // "Orders/003_AL_ORM_O01_NBS_Fully_Populated_1_hl7_translation.fhir" | "Bundle.entry.resource.ofType(Observation).where(code.coding.code = '57723-9').value.coding.code.exists()" } @@ -95,10 +99,78 @@ class RuleEngineIntegrationTest extends Specification { where: testFile | validation - "Orders/001_OML_O21_short.fhir" | "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.value.exists()" + "Orders/001_OML_O21_short.fhir" | "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.where(system = 'urn:ietf:rfc:3986').value.exists()" "Orders/001_OML_O21_short.fhir" | "Bundle.entry.resource.ofType(Observation).where(code.coding.code = '57723-9').value.coding.code.exists()" } + def "validation passes: Message has required receiver id"() { + given: + def fhirPathValidation = "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.where(system = 'urn:ietf:rfc:3986').value.exists()" + def rule = createValidationRule([], [fhirPathValidation]) + + when: + Organization receiverOrganization = new Organization() + receiverOrganization.setId(UUID.randomUUID().toString()) + receiverOrganization + .addIdentifier() + .setSystem("urn:ietf:rfc:3986") + .setValue("simulated-hospital-id") + def bundle = createMessageBundle(receiverOrganization: receiverOrganization) + // for some reason, we need to encode and decode the bundle for resolve() to work + def fhirResource = new HapiFhirResource(fhir.parseResource(fhir.encodeResourceToJson(bundle), Bundle)) + + then: + rule.isValid(fhirResource) + + when: + receiverOrganization = new Organization() + receiverOrganization.setId(UUID.randomUUID().toString()) + receiverOrganization + .addIdentifier() + .setSystem("another-system") + .setValue("simulated-hospital-id") + bundle = createMessageBundle(receiverOrganization: receiverOrganization) + fhirResource = new HapiFhirResource(fhir.parseResource(fhir.encodeResourceToJson(bundle), Bundle)) + + then: + !rule.isValid(fhirResource) + + when: + receiverOrganization = new Organization() + receiverOrganization.setId(UUID.randomUUID().toString()) + receiverOrganization + .addIdentifier() + .setValue("simulated-hospital-id") + bundle = createMessageBundle(receiverOrganization: receiverOrganization) + fhirResource = new HapiFhirResource(fhir.parseResource(fhir.encodeResourceToJson(bundle), Bundle)) + + then: + !rule.isValid(fhirResource) + } + + Bundle createMessageBundle(Map params) { + String messageTypeCode = params.messageTypeCode as String ?: "ORM_O01" + Organization receiverOrganization = params.receiverOrganization as Organization ?: new Organization() + MessageHeader messageHeader = params.messageType as MessageHeader ?: new MessageHeader() + + MessageHeader.MessageDestinationComponent destination = messageHeader.addDestination() + String receiverOrganizationFullUrl = "Organization/" + receiverOrganization.getId() + destination.setReceiver(new Reference(receiverOrganizationFullUrl)) + + Coding eventCoding = new Coding() + eventCoding.setSystem("http://terminology.hl7.org/CodeSystem/v2-0003") + String[] parts = messageTypeCode.split("_") + eventCoding.setCode(parts[1]) + eventCoding.setDisplay(String.format("%s^%s^%s", parts[0], parts[1], messageTypeCode)) + messageHeader.setEvent(eventCoding) + + Bundle bundle = new Bundle() + bundle.setType(Bundle.BundleType.MESSAGE) + bundle.addEntry().setResource(messageHeader) + bundle.addEntry().setFullUrl(receiverOrganizationFullUrl).setResource(receiverOrganization) + return bundle + } + Rule createValidationRule(List ruleConditions, List ruleValidations) { return new ValidationRule( name: "Rule name", From 55bfb475dd68e1cba69151d748093834a138d7ca Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Tue, 2 Apr 2024 14:29:12 -0700 Subject: [PATCH 6/6] Update etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy Co-authored-by: halprin --- .../trustedintermediary/etor/ruleengine/RuleEngineTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy index 50c9a16aa..1e1cf87c4 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy @@ -41,7 +41,7 @@ class RuleEngineTest extends Specification { def "ensureRulesLoaded loads rules only once on multiple threads"() { given: - def threadsNum = 2 + def threadsNum = 10 def iterations = 4 when: