From f5e94fee52bd8e8bb81c1d7f4a79768ca7f5013d Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Tue, 23 Apr 2024 16:10:00 -0700 Subject: [PATCH 01/32] Refactored validation rule engine in preparation for tranformation engine implementation --- .../etor/EtorDomainRegistration.java | 4 +- .../etor/orders/OrderController.java | 2 +- .../etor/ruleengine/Rule.java | 12 +---- .../etor/ruleengine/RuleEngine.java | 43 +++++------------ .../etor/ruleengine/RuleLoader.java | 23 +++++++--- .../etor/ruleengine/TransformationRule.java | 28 +++++++++++ .../etor/ruleengine/ValidationRule.java | 46 +++++++++---------- .../etor/ruleengine/ValidationRuleEngine.java | 18 ++++++++ .../resources/transformation_definitions.json | 9 ++++ ...tions.json => validation_definitions.json} | 0 .../etor/orders/OrderControllerTest.groovy | 2 +- .../RuleEngineIntegrationTest.groovy | 4 +- .../etor/ruleengine/RuleEngineTest.groovy | 16 +++---- .../etor/ruleengine/ValidationRuleTest.groovy | 4 +- 14 files changed, 123 insertions(+), 88 deletions(-) create mode 100644 etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java create mode 100644 etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java create mode 100644 etor/src/main/resources/transformation_definitions.json rename etor/src/main/resources/{rule_definitions.json => validation_definitions.json} (100%) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java index cd7486bea..9c05f95fc 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java @@ -33,8 +33,8 @@ import gov.hhs.cdc.trustedintermediary.etor.results.ResultResponse; import gov.hhs.cdc.trustedintermediary.etor.results.ResultSender; import gov.hhs.cdc.trustedintermediary.etor.results.SendResultUseCase; -import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleEngine; import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleLoader; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRuleEngine; import gov.hhs.cdc.trustedintermediary.external.database.DatabaseMessageLinkStorage; import gov.hhs.cdc.trustedintermediary.external.database.DatabasePartnerMetadataStorage; import gov.hhs.cdc.trustedintermediary.external.database.DbDao; @@ -133,7 +133,7 @@ public Map> domainRegistra PartnerMetadataConverter.class, HapiPartnerMetadataConverter.getInstance()); // Validation rules ApplicationContext.register(RuleLoader.class, RuleLoader.getInstance()); - ApplicationContext.register(RuleEngine.class, RuleEngine.getInstance()); + ApplicationContext.register(ValidationRuleEngine.class, ValidationRuleEngine.getInstance()); ApplicationContext.register(SendMessageHelper.class, SendMessageHelper.getInstance()); diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java index 203bb6ffe..d13c1ef00 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java @@ -31,7 +31,7 @@ public static OrderController getInstance() { public Order parseOrders(DomainRequest request) throws FhirParseException { logger.logInfo("Parsing orders"); var fhirBundle = fhir.parseResource(request.getBody(), Bundle.class); - ruleEngine.validate(new HapiFhirResource(fhirBundle)); + ruleEngine.runRules(new HapiFhirResource(fhirBundle)); metadata.put(fhirBundle.getId(), EtorMetadataStep.RECEIVED_FROM_REPORT_STREAM); return new HapiOrder(fhirBundle); } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java index 55146028d..1657647b1 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java @@ -12,17 +12,9 @@ public interface Rule { String getDescription(); - /** - * Descriptive message when there's a rule violation Note: When implementing this method, make - * sure that no PII or PHI is included in the message! - */ - String getViolationMessage(); - List getConditions(); - List getValidations(); - - boolean isValid(FhirResource resource); + boolean shouldRun(FhirResource resource); - boolean appliesTo(FhirResource resource); + void runRule(FhirResource resource); } 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 0b72ad3eb..7cd3f9a38 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 @@ -1,27 +1,17 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; -import gov.hhs.cdc.trustedintermediary.wrappers.Logger; -import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; -import javax.inject.Inject; /** Manages the application of rules loaded from a definitions file using the RuleLoader. */ public class RuleEngine { - - private static final RuleEngine INSTANCE = new RuleEngine(); - final List rules = new ArrayList<>(); + RuleLoader ruleLoader; + String ruleDefinitionsFileName; - @Inject Logger logger; - @Inject RuleLoader ruleLoader; - - private RuleEngine() {} - - public static RuleEngine getInstance() { - return INSTANCE; + RuleEngine(RuleLoader ruleLoader, String ruleDefinitionsFileName) { + this.ruleLoader = ruleLoader; + this.ruleDefinitionsFileName = ruleDefinitionsFileName; } public void unloadRules() { @@ -32,31 +22,22 @@ public void ensureRulesLoaded() { if (rules.isEmpty()) { synchronized (this) { if (rules.isEmpty()) { - loadRules(); + var parsedRules = ruleLoader.loadRules(ruleDefinitionsFileName); + loadRules(parsedRules); } } } } - private synchronized void loadRules() { - String fileName = "rule_definitions.json"; - try (InputStream ruleDefinitionStream = - getClass().getClassLoader().getResourceAsStream(fileName)) { - assert ruleDefinitionStream != null; - var ruleStream = - new String(ruleDefinitionStream.readAllBytes(), StandardCharsets.UTF_8); - rules.addAll(ruleLoader.loadRules(ruleStream)); - } catch (IOException | RuleLoaderException e) { - logger.logError("Failed to load rules definitions from: " + fileName, e); - } + private synchronized void loadRules(List rules) { + this.rules.addAll(rules); } - public void validate(FhirResource resource) { - logger.logDebug("Validating FHIR resource"); + public void runRules(FhirResource resource) { ensureRulesLoaded(); for (Rule rule : rules) { - if (rule.appliesTo(resource) && !rule.isValid(resource)) { - logger.logWarning("Rule violation: " + rule.getViolationMessage()); + if (rule.shouldRun(resource)) { + rule.runRule(resource); } } } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java index bbfa66ce1..534ef8c50 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java @@ -1,8 +1,12 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; +import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter; import gov.hhs.cdc.trustedintermediary.wrappers.formatter.FormatterProcessingException; import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; import java.util.Map; @@ -12,6 +16,7 @@ public class RuleLoader { private static final RuleLoader INSTANCE = new RuleLoader(); @Inject Formatter formatter; + @Inject Logger logger; private RuleLoader() {} @@ -19,14 +24,18 @@ public static RuleLoader getInstance() { return INSTANCE; } - public List loadRules(String ruleStream) throws RuleLoaderException { - try { - Map> jsonObj = - formatter.convertJsonToObject(ruleStream, new TypeReference<>() {}); + public List loadRules(String fileName) { + try (InputStream ruleDefinitionStream = + getClass().getClassLoader().getResourceAsStream(fileName)) { + assert ruleDefinitionStream != null; + var rulesString = + new String(ruleDefinitionStream.readAllBytes(), StandardCharsets.UTF_8); + Map> jsonObj = + formatter.convertJsonToObject(rulesString, new TypeReference<>() {}); return jsonObj.getOrDefault("rules", Collections.emptyList()); - } catch (FormatterProcessingException e) { - throw new RuleLoaderException( - "Failed to load rules definitions for provided stream", e); + } catch (IOException | FormatterProcessingException e) { + logger.logError("Failed to load rules definitions from: " + fileName, e); + return Collections.emptyList(); } } } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java new file mode 100644 index 000000000..b51ee7157 --- /dev/null +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java @@ -0,0 +1,28 @@ +package gov.hhs.cdc.trustedintermediary.etor.ruleengine; + +import java.util.List; + +public class TransformationRule implements Rule { + @Override + public String getName() { + return ""; + } + + @Override + public String getDescription() { + return ""; + } + + @Override + public List getConditions() { + return List.of(); + } + + @Override + public boolean shouldRun(FhirResource resource) { + return false; + } + + @Override + public void runRule(FhirResource resource) {} +} diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java index ecf02cb6f..c16a159ba 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java @@ -49,7 +49,6 @@ public String getDescription() { return description; } - @Override public String getViolationMessage() { return violationMessage; } @@ -59,34 +58,12 @@ public List getConditions() { return conditions; } - @Override public List getValidations() { return validations; } @Override - public boolean isValid(FhirResource resource) { - return validations.stream() - .allMatch( - validation -> { - try { - return fhirEngine.evaluateCondition( - resource.getUnderlyingResource(), validation); - } catch (Exception e) { - logger.logError( - "Rule [" - + name - + "]: " - + "An error occurred while evaluating the validation: " - + validation, - e); - return false; - } - }); - } - - @Override - public boolean appliesTo(FhirResource resource) { + public boolean shouldRun(FhirResource resource) { return conditions.stream() .allMatch( condition -> { @@ -105,4 +82,25 @@ public boolean appliesTo(FhirResource resource) { } }); } + + @Override + public void runRule(FhirResource resource) { + for (String validation : validations) { + try { + boolean isValid = + fhirEngine.evaluateCondition(resource.getUnderlyingResource(), validation); + if (!isValid) { + logger.logWarning("Rule violation: " + violationMessage); + } + } catch (Exception e) { + logger.logError( + "Rule [" + + name + + "]: " + + "An error occurred while evaluating the validation: " + + validation, + e); + } + } + } } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java new file mode 100644 index 000000000..35296a6dc --- /dev/null +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -0,0 +1,18 @@ +package gov.hhs.cdc.trustedintermediary.etor.ruleengine; + +import gov.hhs.cdc.trustedintermediary.wrappers.Logger; +import javax.inject.Inject; + +public class ValidationRuleEngine extends RuleEngine { + @Inject Logger logger; + + private static final ValidationRuleEngine INSTANCE = new ValidationRuleEngine(); + + private ValidationRuleEngine() { + super(RuleLoader.getInstance(), "validation_definitions.json"); + } + + public static ValidationRuleEngine getInstance() { + return INSTANCE; + } +} diff --git a/etor/src/main/resources/transformation_definitions.json b/etor/src/main/resources/transformation_definitions.json new file mode 100644 index 000000000..6064200aa --- /dev/null +++ b/etor/src/main/resources/transformation_definitions.json @@ -0,0 +1,9 @@ +{ + "rules": [ { + "name": "", + "description": "", + "conditions": [ ], + "transformations": [ ] + } + ] +} diff --git a/etor/src/main/resources/rule_definitions.json b/etor/src/main/resources/validation_definitions.json similarity index 100% rename from etor/src/main/resources/rule_definitions.json rename to etor/src/main/resources/validation_definitions.json diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy index 75bdc1bc1..d8a003e64 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy @@ -38,7 +38,7 @@ class OrderControllerTest extends Specification { then: actualBundle == expectedBundle - (1.._) * ruleEngine.validate(_) + (1.._) * ruleEngine.runRules(_) } def "parseOrders registers a metadata step"() { 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 0bc0d13dc..7629ca46a 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 @@ -41,7 +41,7 @@ class RuleEngineIntegrationTest extends Specification { def bundle = new Bundle() when: - engine.validate(new HapiFhirResource(bundle)) + engine.runRules(new HapiFhirResource(bundle)) then: (1.._) * mockLogger.logWarning(_ as String) @@ -53,7 +53,7 @@ class RuleEngineIntegrationTest extends Specification { when: exampleFhirResources.each { resource -> - engine.validate(resource) + engine.runRules(resource) } then: 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 1e1cf87c4..1aa9b71de 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 @@ -66,13 +66,13 @@ class RuleEngineTest extends Specification { mockRuleLoader.loadRules(_ as String) >> { throw exception } when: - ruleEngine.validate(Mock(FhirResource)) + ruleEngine.runRules(Mock(FhirResource)) then: 1 * mockLogger.logError(_ as String, exception) } - def "validate handles logging warning correctly"() { + def 'runRules handles logging warning correctly'() { given: def ruleViolationMessage = "Rule violation message" def fullRuleViolationMessage = "Rule violation: " + ruleViolationMessage @@ -82,25 +82,25 @@ class RuleEngineTest extends Specification { mockRuleLoader.loadRules(_ as String) >> [invalidRule] when: - invalidRule.appliesTo(fhirBundle) >> true + invalidRule.shouldRun(fhirBundle) >> true invalidRule.isValid(fhirBundle) >> false - ruleEngine.validate(fhirBundle) + ruleEngine.runRules(fhirBundle) then: 1 * mockLogger.logWarning(fullRuleViolationMessage) when: - invalidRule.appliesTo(fhirBundle) >> true + invalidRule.shouldRun(fhirBundle) >> true invalidRule.isValid(fhirBundle) >> true - ruleEngine.validate(fhirBundle) + ruleEngine.runRules(fhirBundle) then: 0 * mockLogger.logWarning(fullRuleViolationMessage) when: - invalidRule.appliesTo(fhirBundle) >> false + invalidRule.shouldRun(fhirBundle) >> false invalidRule.isValid(fhirBundle) >> false - ruleEngine.validate(fhirBundle) + ruleEngine.runRules(fhirBundle) then: 0 * mockLogger.logWarning(fullRuleViolationMessage) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy index 95a0b2545..7ed829be9 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy @@ -50,7 +50,7 @@ class ValidationRuleTest extends Specification { ], null) expect: - rule.appliesTo(new FhirResourceMock("resource")) == applies + rule.shouldRun(new FhirResourceMock("resource")) == applies where: conditionResult | applies @@ -67,7 +67,7 @@ class ValidationRuleTest extends Specification { def rule = new ValidationRule(null, null, null, ["condition"], null) when: - def applies = rule.appliesTo(Mock(FhirResource)) + def applies = rule.shouldRun(Mock(FhirResource)) then: 1 * mockLogger.logError(_ as String, _ as Exception) From 28505a5d895a23f4a61f062405908d08b5879a42 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Tue, 23 Apr 2024 16:26:46 -0700 Subject: [PATCH 02/32] Fixed instantiation --- .../trustedintermediary/etor/orders/OrderController.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java index d13c1ef00..4bd37e334 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java @@ -2,7 +2,7 @@ import gov.hhs.cdc.trustedintermediary.domainconnector.DomainRequest; import gov.hhs.cdc.trustedintermediary.etor.metadata.EtorMetadataStep; -import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleEngine; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRuleEngine; import gov.hhs.cdc.trustedintermediary.external.hapi.HapiFhirResource; import gov.hhs.cdc.trustedintermediary.external.hapi.HapiOrder; import gov.hhs.cdc.trustedintermediary.wrappers.FhirParseException; @@ -20,7 +20,7 @@ public class OrderController { @Inject HapiFhir fhir; @Inject Logger logger; @Inject MetricMetadata metadata; - @Inject RuleEngine ruleEngine; + @Inject ValidationRuleEngine validationEngine; private OrderController() {} @@ -31,7 +31,7 @@ public static OrderController getInstance() { public Order parseOrders(DomainRequest request) throws FhirParseException { logger.logInfo("Parsing orders"); var fhirBundle = fhir.parseResource(request.getBody(), Bundle.class); - ruleEngine.runRules(new HapiFhirResource(fhirBundle)); + validationEngine.runRules(new HapiFhirResource(fhirBundle)); metadata.put(fhirBundle.getId(), EtorMetadataStep.RECEIVED_FROM_REPORT_STREAM); return new HapiOrder(fhirBundle); } From bdd568a8eb7fb85bb4e33e2fe8e30bad34a65cbe Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Thu, 25 Apr 2024 10:47:11 -0700 Subject: [PATCH 03/32] Refactored to make Rule interface more generic --- .../etor/ruleengine/Rule.java | 4 ++++ .../etor/ruleengine/RuleLoader.java | 2 +- .../etor/ruleengine/TransformationRule.java | 10 ++++++++++ .../etor/ruleengine/ValidationRule.java | 20 +++++++++---------- .../resources/transformation_definitions.json | 4 ++-- .../resources/validation_definitions.json | 10 +++++----- .../RuleEngineIntegrationTest.groovy | 4 ++-- .../etor/ruleengine/RuleEngineTest.groovy | 2 +- .../etor/ruleengine/RuleLoaderTest.groovy | 6 +----- .../etor/ruleengine/ValidationRuleTest.groovy | 4 ++-- 10 files changed, 38 insertions(+), 28 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java index 1657647b1..e7d27365f 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java @@ -12,8 +12,12 @@ public interface Rule { String getDescription(); + String getMessage(); + List getConditions(); + List getRules(); + boolean shouldRun(FhirResource resource); void runRule(FhirResource resource); diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java index 534ef8c50..0254898fe 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java @@ -32,7 +32,7 @@ public List loadRules(String fileName) { new String(ruleDefinitionStream.readAllBytes(), StandardCharsets.UTF_8); Map> jsonObj = formatter.convertJsonToObject(rulesString, new TypeReference<>() {}); - return jsonObj.getOrDefault("rules", Collections.emptyList()); + return jsonObj.getOrDefault("definitions", Collections.emptyList()); } catch (IOException | FormatterProcessingException e) { logger.logError("Failed to load rules definitions from: " + fileName, e); return Collections.emptyList(); diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java index b51ee7157..499f8bddc 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java @@ -13,11 +13,21 @@ public String getDescription() { return ""; } + @Override + public String getMessage() { + return ""; + } + @Override public List getConditions() { return List.of(); } + @Override + public List getRules() { + return List.of(); + } + @Override public boolean shouldRun(FhirResource resource) { return false; diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java index c16a159ba..ec393c122 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java @@ -16,9 +16,9 @@ public class ValidationRule implements Rule { private final HapiFhir fhirEngine = ApplicationContext.getImplementation(HapiFhir.class); private String name; private String description; - private String violationMessage; + private String message; private List conditions; - private List validations; + private List rules; /** * Do not delete this constructor! It is used for JSON deserialization when loading rules from a @@ -34,9 +34,9 @@ public ValidationRule( List ruleValidations) { name = ruleName; description = ruleDescription; - violationMessage = ruleWarningMessage; + message = ruleWarningMessage; conditions = ruleConditions; - validations = ruleValidations; + rules = ruleValidations; } @Override @@ -49,8 +49,8 @@ public String getDescription() { return description; } - public String getViolationMessage() { - return violationMessage; + public String getMessage() { + return message; } @Override @@ -58,8 +58,8 @@ public List getConditions() { return conditions; } - public List getValidations() { - return validations; + public List getRules() { + return rules; } @Override @@ -85,12 +85,12 @@ public boolean shouldRun(FhirResource resource) { @Override public void runRule(FhirResource resource) { - for (String validation : validations) { + for (String validation : rules) { try { boolean isValid = fhirEngine.evaluateCondition(resource.getUnderlyingResource(), validation); if (!isValid) { - logger.logWarning("Rule violation: " + violationMessage); + logger.logWarning("Rule violation: " + message); } } catch (Exception e) { logger.logError( diff --git a/etor/src/main/resources/transformation_definitions.json b/etor/src/main/resources/transformation_definitions.json index 6064200aa..3347b41e0 100644 --- a/etor/src/main/resources/transformation_definitions.json +++ b/etor/src/main/resources/transformation_definitions.json @@ -1,9 +1,9 @@ { - "rules": [ { + "definitions": [ { "name": "", "description": "", "conditions": [ ], - "transformations": [ ] + "rules": [ ] } ] } diff --git a/etor/src/main/resources/validation_definitions.json b/etor/src/main/resources/validation_definitions.json index aa828918f..f6d05cb1c 100644 --- a/etor/src/main/resources/validation_definitions.json +++ b/etor/src/main/resources/validation_definitions.json @@ -1,21 +1,21 @@ { - "rules": [ { + "definitions": [ { "name": "messageHasRequiredReceiverId", "description": "Message has required receiver id", - "violationMessage": "Message doesn't have required receiver id", + "message": "Message doesn't have required receiver id", "conditions": [ ], - "validations": [ + "rules": [ "Bundle.entry.resource.ofType(MessageHeader).destination.receiver.resolve().identifier.where(system = 'urn:ietf:rfc:3986').value.exists()" ] }, { "name": "ORMHasRequiredCardNumber", "description": "ORM has required Card Number", - "violationMessage": "ORM doesn't have required Card Number", + "message": "ORM doesn't have required Card Number", "conditions": [ "Bundle.entry.resource.ofType(MessageHeader).event.code = 'O01'" ], - "validations": [ + "rules": [ "Bundle.entry.resource.ofType(Observation).where(code.coding.code = '57723-9').value.coding.code.exists()" ] } 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 7629ca46a..8cafcec2f 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 @@ -175,9 +175,9 @@ class RuleEngineIntegrationTest extends Specification { return new ValidationRule( name: "Rule name", description: "Rule description", - violationMessage: "Rule warning message", + message: "Rule warning message", conditions: ruleConditions, - validations: ruleValidations, + rules: ruleValidations, ) } 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 1aa9b71de..f2194c5e7 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 @@ -78,7 +78,7 @@ class RuleEngineTest extends Specification { def fullRuleViolationMessage = "Rule violation: " + ruleViolationMessage def fhirBundle = Mock(FhirResource) def invalidRule = Mock(Rule) - invalidRule.getViolationMessage() >> ruleViolationMessage + invalidRule.getMessage() >> ruleViolationMessage mockRuleLoader.loadRules(_ as String) >> [invalidRule] when: diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy index 6f3245876..ca093b83c 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy @@ -6,10 +6,6 @@ import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter import spock.lang.Specification -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.Paths - class RuleLoaderTest extends Specification { String fileContents @@ -47,7 +43,7 @@ class RuleLoaderTest extends Specification { ValidationRule rule = rules.get(0) as ValidationRule rule.getName() == "patientName" rule.getConditions() == ["Patient.name.exists()"] - rule.getValidations() == [ + rule.getRules() == [ "Patient.name.where(use='usual').given.exists()" ] } diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy index 7ed829be9..701493bea 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy @@ -33,9 +33,9 @@ class ValidationRuleTest extends Specification { then: rule.getName() == ruleName rule.getDescription() == ruleDescription - rule.getViolationMessage() == ruleWarningMessage + rule.getMessage() == ruleWarningMessage rule.getConditions() == conditions - rule.getValidations() == validations + rule.getRules() == validations } def "appliesTo returns expected boolean depending on conditions"() { From f65ceded8fc42f25ae5429a122e69eb42fa6229b Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Thu, 25 Apr 2024 11:51:59 -0700 Subject: [PATCH 04/32] Fixed tests --- .../etor/orders/OrderControllerTest.groovy | 6 +++--- ...st.groovy => ValidationRuleEngineIntegrationTest.groovy} | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) rename etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/{RuleEngineIntegrationTest.groovy => ValidationRuleEngineIntegrationTest.groovy} (97%) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy index d8a003e64..034072f68 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy @@ -3,7 +3,7 @@ package gov.hhs.cdc.trustedintermediary.etor.orders import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.domainconnector.DomainRequest import gov.hhs.cdc.trustedintermediary.etor.metadata.EtorMetadataStep -import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleEngine +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRuleEngine import gov.hhs.cdc.trustedintermediary.external.hapi.HapiMessageHelper import gov.hhs.cdc.trustedintermediary.wrappers.FhirParseException import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir @@ -12,14 +12,14 @@ import org.hl7.fhir.r4.model.Bundle import spock.lang.Specification class OrderControllerTest extends Specification { - def ruleEngine = Mock(RuleEngine) + def ruleEngine = Mock(ValidationRuleEngine) def setup() { TestApplicationContext.reset() TestApplicationContext.init() TestApplicationContext.register(OrderController, OrderController.getInstance()) TestApplicationContext.register(MetricMetadata, Mock(MetricMetadata)) - TestApplicationContext.register(RuleEngine, ruleEngine) + TestApplicationContext.register(ValidationRuleEngine, ruleEngine) TestApplicationContext.register(HapiMessageHelper, HapiMessageHelper.getInstance()) } 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/ValidationRuleEngineIntegrationTest.groovy similarity index 97% rename from etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineIntegrationTest.groovy rename to etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy index 8cafcec2f..ed78ae6ce 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/ValidationRuleEngineIntegrationTest.groovy @@ -17,10 +17,10 @@ import spock.lang.Specification import java.nio.file.Files import java.nio.file.Path -class RuleEngineIntegrationTest extends Specification { +class ValidationRuleEngineIntegrationTest extends Specification { def testExampleFilesPath = "../examples/Test" def fhir = HapiFhirImplementation.getInstance() - def engine = RuleEngine.getInstance() + def engine = ValidationRuleEngine.getInstance() def mockLogger = Mock(Logger) def setup() { @@ -29,7 +29,7 @@ class RuleEngineIntegrationTest extends Specification { TestApplicationContext.register(Formatter, Jackson.getInstance()) TestApplicationContext.register(HapiFhir, fhir) - TestApplicationContext.register(RuleEngine, engine) + TestApplicationContext.register(ValidationRuleEngine, engine) TestApplicationContext.register(RuleLoader, RuleLoader.getInstance()) TestApplicationContext.register(Logger, mockLogger) From 0cf56404ff0584dcd78de356ba664e96d450e8f3 Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Thu, 25 Apr 2024 15:50:56 -0400 Subject: [PATCH 05/32] Update transformation_definitions.json Added dummy definitions from design discussion --- .../resources/transformation_definitions.json | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/etor/src/main/resources/transformation_definitions.json b/etor/src/main/resources/transformation_definitions.json index 3347b41e0..ee33f9db4 100644 --- a/etor/src/main/resources/transformation_definitions.json +++ b/etor/src/main/resources/transformation_definitions.json @@ -1,9 +1,37 @@ { "definitions": [ { - "name": "", - "description": "", - "conditions": [ ], - "rules": [ ] + "name": "flattenOBR", + "description": "Flatten OBRs", + "message": "ORM doesn't have required Card Number", + "conditions": [ + "Bundle.entry.resource.ofType(MessageHeader).destination.sender = 'CA'" + ], + "rules": [ + { + "name": "FlattenOBRTransformation", + "args": {} + }, + { + "name": "updateFhirPathValue", + "args": { + "fhirPathName": "receivingFacilityName", + "value": "CDPH" + } + }, + { + "name": "updateFhirPathValue", + "args": { + "fhirPathName": "Bundle/MSH5", + "value": "EPIC" + } + }, + { + "name": "removeFhirPathValue", + "args": { + "fhirPathName": "Bundle/PID-3.4" + } + } + ] } ] } From 86313349aa4effa6759f3bbf62fbb21928ec80c6 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Thu, 25 Apr 2024 15:27:35 -0700 Subject: [PATCH 06/32] Added missing override tags --- .../cdc/trustedintermediary/etor/ruleengine/ValidationRule.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java index ec393c122..cc17e2746 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java @@ -49,6 +49,7 @@ public String getDescription() { return description; } + @Override public String getMessage() { return message; } @@ -58,6 +59,7 @@ public List getConditions() { return conditions; } + @Override public List getRules() { return rules; } From 6214e848cc9facb06ac05fb4c6c9fc40a90dd92a Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Thu, 25 Apr 2024 15:34:47 -0700 Subject: [PATCH 07/32] Fixed arg naming --- .../etor/ruleengine/ValidationRule.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java index cc17e2746..74870b52f 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java @@ -29,14 +29,14 @@ public ValidationRule() {} public ValidationRule( String ruleName, String ruleDescription, - String ruleWarningMessage, + String ruleMessage, List ruleConditions, - List ruleValidations) { + List ruleActions) { name = ruleName; description = ruleDescription; - message = ruleWarningMessage; + message = ruleMessage; conditions = ruleConditions; - rules = ruleValidations; + rules = ruleActions; } @Override From 4756364b0d48b5bd13126d5bed6b9f305edf22da Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Thu, 25 Apr 2024 16:37:49 -0700 Subject: [PATCH 08/32] Use generics to deserialize correct type of Rule implementation --- .../etor/ruleengine/RuleEngine.java | 18 +++++----- .../etor/ruleengine/RuleLoader.java | 4 +-- .../etor/ruleengine/ValidationRuleEngine.java | 9 ++--- .../resources/transformation_definitions.json | 34 ++++--------------- .../etor/ruleengine/RuleEngineTest.groovy | 7 ++-- 5 files changed, 25 insertions(+), 47 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 7cd3f9a38..daaf3b35d 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 @@ -4,14 +4,16 @@ import java.util.List; /** Manages the application of rules loaded from a definitions file using the RuleLoader. */ -public class RuleEngine { - final List rules = new ArrayList<>(); - RuleLoader ruleLoader; - String ruleDefinitionsFileName; +public class RuleEngine { + final List rules = new ArrayList<>(); + private final RuleLoader ruleLoader; + private final String ruleDefinitionsFileName; + private final Class ruleClass; - RuleEngine(RuleLoader ruleLoader, String ruleDefinitionsFileName) { + RuleEngine(RuleLoader ruleLoader, String ruleDefinitionsFileName, Class ruleClass) { this.ruleLoader = ruleLoader; this.ruleDefinitionsFileName = ruleDefinitionsFileName; + this.ruleClass = ruleClass; } public void unloadRules() { @@ -22,20 +24,20 @@ public void ensureRulesLoaded() { if (rules.isEmpty()) { synchronized (this) { if (rules.isEmpty()) { - var parsedRules = ruleLoader.loadRules(ruleDefinitionsFileName); + List parsedRules = ruleLoader.loadRules(ruleDefinitionsFileName, ruleClass); loadRules(parsedRules); } } } } - private synchronized void loadRules(List rules) { + private synchronized void loadRules(List rules) { this.rules.addAll(rules); } public void runRules(FhirResource resource) { ensureRulesLoaded(); - for (Rule rule : rules) { + for (T rule : rules) { if (rule.shouldRun(resource)) { rule.runRule(resource); } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java index 0254898fe..dcef4b87a 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java @@ -24,13 +24,13 @@ public static RuleLoader getInstance() { return INSTANCE; } - public List loadRules(String fileName) { + public List loadRules(String fileName, Class ruleClass) { try (InputStream ruleDefinitionStream = getClass().getClassLoader().getResourceAsStream(fileName)) { assert ruleDefinitionStream != null; var rulesString = new String(ruleDefinitionStream.readAllBytes(), StandardCharsets.UTF_8); - Map> jsonObj = + Map> jsonObj = formatter.convertJsonToObject(rulesString, new TypeReference<>() {}); return jsonObj.getOrDefault("definitions", Collections.emptyList()); } catch (IOException | FormatterProcessingException e) { diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index 35296a6dc..a6def0693 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -1,15 +1,10 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; -import gov.hhs.cdc.trustedintermediary.wrappers.Logger; -import javax.inject.Inject; - -public class ValidationRuleEngine extends RuleEngine { - @Inject Logger logger; - +public class ValidationRuleEngine extends RuleEngine { private static final ValidationRuleEngine INSTANCE = new ValidationRuleEngine(); private ValidationRuleEngine() { - super(RuleLoader.getInstance(), "validation_definitions.json"); + super(RuleLoader.getInstance(), "validation_definitions.json", ValidationRule.class); } public static ValidationRuleEngine getInstance() { diff --git a/etor/src/main/resources/transformation_definitions.json b/etor/src/main/resources/transformation_definitions.json index ee33f9db4..b608d1b2c 100644 --- a/etor/src/main/resources/transformation_definitions.json +++ b/etor/src/main/resources/transformation_definitions.json @@ -1,35 +1,13 @@ { "definitions": [ { - "name": "flattenOBR", - "description": "Flatten OBRs", - "message": "ORM doesn't have required Card Number", - "conditions": [ - "Bundle.entry.resource.ofType(MessageHeader).destination.sender = 'CA'" - ], + "name": "Rule name", + "description": "Rule description", + "message": "Log message", + "conditions": [ ], "rules": [ { - "name": "FlattenOBRTransformation", - "args": {} - }, - { - "name": "updateFhirPathValue", - "args": { - "fhirPathName": "receivingFacilityName", - "value": "CDPH" - } - }, - { - "name": "updateFhirPathValue", - "args": { - "fhirPathName": "Bundle/MSH5", - "value": "EPIC" - } - }, - { - "name": "removeFhirPathValue", - "args": { - "fhirPathName": "Bundle/PID-3.4" - } + "name": "Transformation method name", + "args": { } } ] } 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 f2194c5e7..b2250dbfc 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 @@ -1,11 +1,13 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext +import gov.hhs.cdc.trustedintermediary.external.jackson.Jackson import gov.hhs.cdc.trustedintermediary.wrappers.Logger +import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter import spock.lang.Specification class RuleEngineTest extends Specification { - def ruleEngine = RuleEngine.getInstance() + def ruleEngine = ValidationRuleEngine.getInstance() def mockRuleLoader = Mock(RuleLoader) def mockLogger = Mock(Logger) @@ -16,7 +18,8 @@ class RuleEngineTest extends Specification { TestApplicationContext.init() TestApplicationContext.register(RuleLoader, mockRuleLoader) TestApplicationContext.register(Logger, mockLogger) - TestApplicationContext.register(RuleEngine, ruleEngine) + TestApplicationContext.register(ValidationRuleEngine, ruleEngine) + TestApplicationContext.register(Formatter, Jackson.getInstance()) TestApplicationContext.injectRegisteredImplementations() } From 1de28efa289b3d8ba2b61bc4f24ab44e64be5b0d Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 26 Apr 2024 08:16:01 -0700 Subject: [PATCH 09/32] Converted RuleEngine back into a singleton and added parameters --- .../etor/EtorDomainRegistration.java | 6 ++- .../etor/ruleengine/IRuleEngine.java | 10 +++++ .../etor/ruleengine/RuleEngine.java | 40 ++++++++++--------- .../etor/ruleengine/RuleLoader.java | 2 +- .../etor/ruleengine/ValidationRuleEngine.java | 12 +----- .../etor/ruleengine/RuleEngineTest.groovy | 20 ++++++---- 6 files changed, 51 insertions(+), 39 deletions(-) create mode 100644 etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/IRuleEngine.java diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java index c50aa3062..ded332cf8 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java @@ -33,7 +33,9 @@ import gov.hhs.cdc.trustedintermediary.etor.results.ResultResponse; import gov.hhs.cdc.trustedintermediary.etor.results.ResultSender; import gov.hhs.cdc.trustedintermediary.etor.results.SendResultUseCase; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleEngine; import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleLoader; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRule; import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRuleEngine; import gov.hhs.cdc.trustedintermediary.external.database.DatabaseMessageLinkStorage; import gov.hhs.cdc.trustedintermediary.external.database.DatabasePartnerMetadataStorage; @@ -133,7 +135,9 @@ public Map> domainRegistra PartnerMetadataConverter.class, HapiPartnerMetadataConverter.getInstance()); // Validation rules ApplicationContext.register(RuleLoader.class, RuleLoader.getInstance()); - ApplicationContext.register(ValidationRuleEngine.class, ValidationRuleEngine.getInstance()); + ApplicationContext.register( + ValidationRuleEngine.class, + RuleEngine.getInstance("validation_definitions.json", ValidationRule.class)); ApplicationContext.register(SendMessageHelper.class, SendMessageHelper.getInstance()); diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/IRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/IRuleEngine.java new file mode 100644 index 000000000..418460f5e --- /dev/null +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/IRuleEngine.java @@ -0,0 +1,10 @@ +package gov.hhs.cdc.trustedintermediary.etor.ruleengine; + +public interface IRuleEngine { + + void unloadRules(); + + void ensureRulesLoaded(); + + void runRules(FhirResource resource); +} 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 daaf3b35d..d51a49157 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 @@ -2,42 +2,46 @@ import java.util.ArrayList; import java.util.List; +import javax.inject.Inject; /** Manages the application of rules loaded from a definitions file using the RuleLoader. */ -public class RuleEngine { - final List rules = new ArrayList<>(); - private final RuleLoader ruleLoader; - private final String ruleDefinitionsFileName; - private final Class ruleClass; - - RuleEngine(RuleLoader ruleLoader, String ruleDefinitionsFileName, Class ruleClass) { - this.ruleLoader = ruleLoader; - this.ruleDefinitionsFileName = ruleDefinitionsFileName; - this.ruleClass = ruleClass; +public class RuleEngine implements IRuleEngine { + private Class ruleClass; + private String ruleDefinitionsFileName; + final List rules = new ArrayList<>(); + + private static final RuleEngine INSTANCE = new RuleEngine(); + + @Inject RuleLoader ruleLoader; + + public static RuleEngine getInstance(String ruleDefinitionsFileName, Class ruleClass) { + INSTANCE.ruleDefinitionsFileName = ruleDefinitionsFileName; + INSTANCE.ruleClass = ruleClass; + return INSTANCE; } + private RuleEngine() {} + public void unloadRules() { rules.clear(); } public void ensureRulesLoaded() { - if (rules.isEmpty()) { - synchronized (this) { - if (rules.isEmpty()) { - List parsedRules = ruleLoader.loadRules(ruleDefinitionsFileName, ruleClass); - loadRules(parsedRules); - } + synchronized (this) { + if (rules.isEmpty()) { + List parsedRules = ruleLoader.loadRules(ruleDefinitionsFileName, ruleClass); + loadRules(parsedRules); } } } - private synchronized void loadRules(List rules) { + private synchronized void loadRules(List rules) { this.rules.addAll(rules); } public void runRules(FhirResource resource) { ensureRulesLoaded(); - for (T rule : rules) { + for (Rule rule : rules) { if (rule.shouldRun(resource)) { rule.runRule(resource); } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java index dcef4b87a..77e4de1f4 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java @@ -24,7 +24,7 @@ public static RuleLoader getInstance() { return INSTANCE; } - public List loadRules(String fileName, Class ruleClass) { + public List loadRules(String fileName, Class ruleClass) { try (InputStream ruleDefinitionStream = getClass().getClassLoader().getResourceAsStream(fileName)) { assert ruleDefinitionStream != null; diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index a6def0693..003217165 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -1,13 +1,3 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; -public class ValidationRuleEngine extends RuleEngine { - private static final ValidationRuleEngine INSTANCE = new ValidationRuleEngine(); - - private ValidationRuleEngine() { - super(RuleLoader.getInstance(), "validation_definitions.json", ValidationRule.class); - } - - public static ValidationRuleEngine getInstance() { - return INSTANCE; - } -} +public interface ValidationRuleEngine extends IRuleEngine {} 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 b2250dbfc..951486d60 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 @@ -7,7 +7,7 @@ import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter import spock.lang.Specification class RuleEngineTest extends Specification { - def ruleEngine = ValidationRuleEngine.getInstance() + def ruleEngine = RuleEngine.getInstance("validation_definitions.json", ValidationRule.class) def mockRuleLoader = Mock(RuleLoader) def mockLogger = Mock(Logger) @@ -18,18 +18,22 @@ class RuleEngineTest extends Specification { TestApplicationContext.init() TestApplicationContext.register(RuleLoader, mockRuleLoader) TestApplicationContext.register(Logger, mockLogger) - TestApplicationContext.register(ValidationRuleEngine, ruleEngine) - TestApplicationContext.register(Formatter, Jackson.getInstance()) + TestApplicationContext.register(RuleEngine, ruleEngine) TestApplicationContext.injectRegisteredImplementations() } def "ensureRulesLoaded happy path"() { + given: + def mockRuleLoader = Mock(RuleLoader) + TestApplicationContext.register(RuleLoader, mockRuleLoader) + TestApplicationContext.injectRegisteredImplementations() + when: ruleEngine.ensureRulesLoaded() then: - 1 * mockRuleLoader.loadRules(_ as String) >> [Mock(Rule)] + 1 * mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> [Mock(Rule)] ruleEngine.rules.size() == 1 } @@ -39,7 +43,7 @@ class RuleEngineTest extends Specification { ruleEngine.ensureRulesLoaded() // Call twice to test if rules are loaded only once then: - 1 * mockRuleLoader.loadRules(_ as String) >> [Mock(Rule)] + 1 * mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> [Mock(Rule)] } def "ensureRulesLoaded loads rules only once on multiple threads"() { @@ -60,13 +64,13 @@ class RuleEngineTest extends Specification { threads*.join() then: - 1 * mockRuleLoader.loadRules(_ as String) >> [Mock(Rule)] + 1 * mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> [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()) - mockRuleLoader.loadRules(_ as String) >> { throw exception } + mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> { throw exception } when: ruleEngine.runRules(Mock(FhirResource)) @@ -82,7 +86,7 @@ class RuleEngineTest extends Specification { def fhirBundle = Mock(FhirResource) def invalidRule = Mock(Rule) invalidRule.getMessage() >> ruleViolationMessage - mockRuleLoader.loadRules(_ as String) >> [invalidRule] + mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> [invalidRule] when: invalidRule.shouldRun(fhirBundle) >> true From 5c8073bdf9cea5fef97d0e16adda4cb09a69500e Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 26 Apr 2024 10:07:07 -0700 Subject: [PATCH 10/32] Refactored to have a separate implementation for ValidationRuleEngine --- .../etor/EtorDomainRegistration.java | 4 +- .../etor/ruleengine/IRuleEngine.java | 10 ---- .../etor/ruleengine/RuleEngine.java | 48 ++----------------- .../etor/ruleengine/RuleLoader.java | 2 +- .../etor/ruleengine/ValidationRuleEngine.java | 47 +++++++++++++++++- ...ValidationRuleEngineIntegrationTest.groovy | 2 +- 6 files changed, 53 insertions(+), 60 deletions(-) delete mode 100644 etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/IRuleEngine.java diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java index ded332cf8..d839a91e3 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java @@ -33,9 +33,7 @@ import gov.hhs.cdc.trustedintermediary.etor.results.ResultResponse; import gov.hhs.cdc.trustedintermediary.etor.results.ResultSender; import gov.hhs.cdc.trustedintermediary.etor.results.SendResultUseCase; -import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleEngine; import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleLoader; -import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRule; import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRuleEngine; import gov.hhs.cdc.trustedintermediary.external.database.DatabaseMessageLinkStorage; import gov.hhs.cdc.trustedintermediary.external.database.DatabasePartnerMetadataStorage; @@ -137,7 +135,7 @@ public Map> domainRegistra ApplicationContext.register(RuleLoader.class, RuleLoader.getInstance()); ApplicationContext.register( ValidationRuleEngine.class, - RuleEngine.getInstance("validation_definitions.json", ValidationRule.class)); + ValidationRuleEngine.getInstance("validation_definitions.json")); ApplicationContext.register(SendMessageHelper.class, SendMessageHelper.getInstance()); diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/IRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/IRuleEngine.java deleted file mode 100644 index 418460f5e..000000000 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/IRuleEngine.java +++ /dev/null @@ -1,10 +0,0 @@ -package gov.hhs.cdc.trustedintermediary.etor.ruleengine; - -public interface IRuleEngine { - - void unloadRules(); - - void ensureRulesLoaded(); - - void runRules(FhirResource resource); -} 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 d51a49157..917acfa32 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 @@ -1,50 +1,10 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; -import java.util.ArrayList; -import java.util.List; -import javax.inject.Inject; - /** Manages the application of rules loaded from a definitions file using the RuleLoader. */ -public class RuleEngine implements IRuleEngine { - private Class ruleClass; - private String ruleDefinitionsFileName; - final List rules = new ArrayList<>(); - - private static final RuleEngine INSTANCE = new RuleEngine(); - - @Inject RuleLoader ruleLoader; - - public static RuleEngine getInstance(String ruleDefinitionsFileName, Class ruleClass) { - INSTANCE.ruleDefinitionsFileName = ruleDefinitionsFileName; - INSTANCE.ruleClass = ruleClass; - return INSTANCE; - } - - private RuleEngine() {} - - public void unloadRules() { - rules.clear(); - } - - public void ensureRulesLoaded() { - synchronized (this) { - if (rules.isEmpty()) { - List parsedRules = ruleLoader.loadRules(ruleDefinitionsFileName, ruleClass); - loadRules(parsedRules); - } - } - } +public interface RuleEngine { + void unloadRules(); - private synchronized void loadRules(List rules) { - this.rules.addAll(rules); - } + void ensureRulesLoaded(); - public void runRules(FhirResource resource) { - ensureRulesLoaded(); - for (Rule rule : rules) { - if (rule.shouldRun(resource)) { - rule.runRule(resource); - } - } - } + void runRules(FhirResource resource); } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java index 77e4de1f4..08c850687 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java @@ -24,7 +24,7 @@ public static RuleLoader getInstance() { return INSTANCE; } - public List loadRules(String fileName, Class ruleClass) { + public List loadRules(String fileName, Class clazz) { try (InputStream ruleDefinitionStream = getClass().getClassLoader().getResourceAsStream(fileName)) { assert ruleDefinitionStream != null; diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index 003217165..0a26acafa 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -1,3 +1,48 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; -public interface ValidationRuleEngine extends IRuleEngine {} +import java.util.ArrayList; +import java.util.List; +import javax.inject.Inject; + +public class ValidationRuleEngine implements RuleEngine { + private String ruleDefinitionsFileName; + final List rules = new ArrayList<>(); + + private static final ValidationRuleEngine INSTANCE = new ValidationRuleEngine(); + + @Inject RuleLoader ruleLoader; + + public static ValidationRuleEngine getInstance(String ruleDefinitionsFileName) { + INSTANCE.ruleDefinitionsFileName = ruleDefinitionsFileName; + return INSTANCE; + } + + private ValidationRuleEngine() {} + + public void unloadRules() { + rules.clear(); + } + + public void ensureRulesLoaded() { + synchronized (this) { + if (rules.isEmpty()) { + List parsedRules = + ruleLoader.loadRules(ruleDefinitionsFileName, ValidationRule.class); + loadRules(parsedRules); + } + } + } + + private synchronized void loadRules(List rules) { + this.rules.addAll(rules); + } + + public void runRules(FhirResource resource) { + ensureRulesLoaded(); + for (ValidationRule rule : rules) { + if (rule.shouldRun(resource)) { + rule.runRule(resource); + } + } + } +} diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy index ed78ae6ce..00a47f87f 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy @@ -20,7 +20,7 @@ import java.nio.file.Path class ValidationRuleEngineIntegrationTest extends Specification { def testExampleFilesPath = "../examples/Test" def fhir = HapiFhirImplementation.getInstance() - def engine = ValidationRuleEngine.getInstance() + def engine = ValidationRuleEngine.getInstance("validation_definitions.json") def mockLogger = Mock(Logger) def setup() { From 45a3bca1574a6aa8827811f671d1cf95950fce1b Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 26 Apr 2024 13:36:24 -0700 Subject: [PATCH 11/32] Refactored to pass TypeReference and fix mapping issue --- .../cdc/trustedintermediary/etor/ruleengine/RuleLoader.java | 5 +++-- .../etor/ruleengine/ValidationRuleEngine.java | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java index 08c850687..7745dc95c 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java @@ -24,14 +24,15 @@ public static RuleLoader getInstance() { return INSTANCE; } - public List loadRules(String fileName, Class clazz) { + public List loadRules( + String fileName, TypeReference>> typeReference) { try (InputStream ruleDefinitionStream = getClass().getClassLoader().getResourceAsStream(fileName)) { assert ruleDefinitionStream != null; var rulesString = new String(ruleDefinitionStream.readAllBytes(), StandardCharsets.UTF_8); Map> jsonObj = - formatter.convertJsonToObject(rulesString, new TypeReference<>() {}); + formatter.convertJsonToObject(rulesString, typeReference); return jsonObj.getOrDefault("definitions", Collections.emptyList()); } catch (IOException | FormatterProcessingException e) { logger.logError("Failed to load rules definitions from: " + fileName, e); diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index 0a26acafa..e505e3c22 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -1,5 +1,6 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; +import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference; import java.util.ArrayList; import java.util.List; import javax.inject.Inject; @@ -27,7 +28,7 @@ public void ensureRulesLoaded() { synchronized (this) { if (rules.isEmpty()) { List parsedRules = - ruleLoader.loadRules(ruleDefinitionsFileName, ValidationRule.class); + ruleLoader.loadRules(ruleDefinitionsFileName, new TypeReference<>() {}); loadRules(parsedRules); } } From d5490e89b4422da1ac5772d7bb7dbc0940f50491 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 26 Apr 2024 13:49:58 -0700 Subject: [PATCH 12/32] Added missing override tags --- .../etor/ruleengine/ValidationRuleEngine.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index e505e3c22..1f2d77c45 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -20,10 +20,12 @@ public static ValidationRuleEngine getInstance(String ruleDefinitionsFileName) { private ValidationRuleEngine() {} + @Override public void unloadRules() { rules.clear(); } + @Override public void ensureRulesLoaded() { synchronized (this) { if (rules.isEmpty()) { @@ -34,10 +36,7 @@ public void ensureRulesLoaded() { } } - private synchronized void loadRules(List rules) { - this.rules.addAll(rules); - } - + @Override public void runRules(FhirResource resource) { ensureRulesLoaded(); for (ValidationRule rule : rules) { @@ -46,4 +45,8 @@ public void runRules(FhirResource resource) { } } } + + private synchronized void loadRules(List rules) { + this.rules.addAll(rules); + } } From 8e7398c77899f966b49a86910b6feafcb7e81d6b Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 26 Apr 2024 14:12:54 -0700 Subject: [PATCH 13/32] Fixed tests in ValidationRuleEngineIntegrationTest --- ...ValidationRuleEngineIntegrationTest.groovy | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy index 00a47f87f..52a91350a 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy @@ -64,22 +64,25 @@ class ValidationRuleEngineIntegrationTest extends Specification { given: def fhirResource = getExampleFhirResource("e2e/orders/001_OML_O21_short.fhir") def validation = "Bundle.entry.resource.ofType(MessageHeader).focus.resolve().category.exists()" - def rule = createValidationRule([], [validation]) + Rule rule = createValidationRule([], [validation]) when: - def applies = rule.isValid(fhirResource) + rule.runRule(fhirResource) then: - applies + 0 * mockLogger.logWarning(_ as String) + 0 * mockLogger.logError(_ as String, _ as Throwable) } def "validation rules pass for test files"() { given: def fhirResource = getExampleFhirResource(testFile) def rule = createValidationRule([], [validation]) + 0 * mockLogger.logWarning(_ as String) + 0 * mockLogger.logError(_ as String, _ as Throwable) expect: - rule.isValid(fhirResource) + rule.runRule(fhirResource) where: testFile | validation @@ -93,9 +96,11 @@ class ValidationRuleEngineIntegrationTest extends Specification { given: def fhirResource = getExampleFhirResource(testFile) def rule = createValidationRule([], [validation]) + 1 * mockLogger.logWarning(_ as String) + 0 * mockLogger.logError(_ as String, _ as Throwable) expect: - !rule.isValid(fhirResource) + rule.runRule(fhirResource) where: testFile | validation @@ -118,9 +123,11 @@ class ValidationRuleEngineIntegrationTest extends Specification { 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)) + rule.runRule(fhirResource) then: - rule.isValid(fhirResource) + 0 * mockLogger.logWarning(_ as String) + 0 * mockLogger.logError(_ as String, _ as Throwable) when: receiverOrganization = new Organization() @@ -131,9 +138,11 @@ class ValidationRuleEngineIntegrationTest extends Specification { .setValue("simulated-hospital-id") bundle = createMessageBundle(receiverOrganization: receiverOrganization) fhirResource = new HapiFhirResource(fhir.parseResource(fhir.encodeResourceToJson(bundle), Bundle)) + rule.runRule(fhirResource) then: - !rule.isValid(fhirResource) + 1 * mockLogger.logWarning(_ as String) + 0 * mockLogger.logError(_ as String, _ as Throwable) when: receiverOrganization = new Organization() @@ -143,9 +152,11 @@ class ValidationRuleEngineIntegrationTest extends Specification { .setValue("simulated-hospital-id") bundle = createMessageBundle(receiverOrganization: receiverOrganization) fhirResource = new HapiFhirResource(fhir.parseResource(fhir.encodeResourceToJson(bundle), Bundle)) + rule.runRule(fhirResource) then: - !rule.isValid(fhirResource) + 1 * mockLogger.logWarning(_ as String) + 0 * mockLogger.logError(_ as String, _ as Throwable) } Bundle createMessageBundle(Map params) { From dbd80438ef865af8c3c459e85b88392a92284b08 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 26 Apr 2024 14:24:22 -0700 Subject: [PATCH 14/32] Fixed more tests --- ...ValidationRuleEngineIntegrationTest.groovy | 12 ++++----- ...groovy => ValidationRuleEngineTest.groovy} | 13 +++++----- .../etor/ruleengine/ValidationRuleTest.groovy | 25 ++++++++++++------- 3 files changed, 28 insertions(+), 22 deletions(-) rename etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/{RuleEngineTest.groovy => ValidationRuleEngineTest.groovy} (86%) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy index 52a91350a..3cef1191e 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy @@ -71,7 +71,7 @@ class ValidationRuleEngineIntegrationTest extends Specification { then: 0 * mockLogger.logWarning(_ as String) - 0 * mockLogger.logError(_ as String, _ as Throwable) + 0 * mockLogger.logError(_ as String, _ as Exception) } def "validation rules pass for test files"() { @@ -79,7 +79,7 @@ class ValidationRuleEngineIntegrationTest extends Specification { def fhirResource = getExampleFhirResource(testFile) def rule = createValidationRule([], [validation]) 0 * mockLogger.logWarning(_ as String) - 0 * mockLogger.logError(_ as String, _ as Throwable) + 0 * mockLogger.logError(_ as String, _ as Exception) expect: rule.runRule(fhirResource) @@ -97,7 +97,7 @@ class ValidationRuleEngineIntegrationTest extends Specification { def fhirResource = getExampleFhirResource(testFile) def rule = createValidationRule([], [validation]) 1 * mockLogger.logWarning(_ as String) - 0 * mockLogger.logError(_ as String, _ as Throwable) + 0 * mockLogger.logError(_ as String, _ as Exception) expect: rule.runRule(fhirResource) @@ -127,7 +127,7 @@ class ValidationRuleEngineIntegrationTest extends Specification { then: 0 * mockLogger.logWarning(_ as String) - 0 * mockLogger.logError(_ as String, _ as Throwable) + 0 * mockLogger.logError(_ as String, _ as Exception) when: receiverOrganization = new Organization() @@ -142,7 +142,7 @@ class ValidationRuleEngineIntegrationTest extends Specification { then: 1 * mockLogger.logWarning(_ as String) - 0 * mockLogger.logError(_ as String, _ as Throwable) + 0 * mockLogger.logError(_ as String, _ as Exception) when: receiverOrganization = new Organization() @@ -156,7 +156,7 @@ class ValidationRuleEngineIntegrationTest extends Specification { then: 1 * mockLogger.logWarning(_ as String) - 0 * mockLogger.logError(_ as String, _ as Throwable) + 0 * mockLogger.logError(_ as String, _ as Exception) } Bundle createMessageBundle(Map params) { 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/ValidationRuleEngineTest.groovy similarity index 86% rename from etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleEngineTest.groovy rename to etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index 951486d60..cf20bec19 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/ValidationRuleEngineTest.groovy @@ -1,13 +1,12 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext -import gov.hhs.cdc.trustedintermediary.external.jackson.Jackson import gov.hhs.cdc.trustedintermediary.wrappers.Logger -import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter +import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference import spock.lang.Specification -class RuleEngineTest extends Specification { - def ruleEngine = RuleEngine.getInstance("validation_definitions.json", ValidationRule.class) +class ValidationRuleEngineTest extends Specification { + def ruleEngine = ValidationRuleEngine.getInstance("validation_definitions.json") def mockRuleLoader = Mock(RuleLoader) def mockLogger = Mock(Logger) @@ -70,7 +69,7 @@ class RuleEngineTest extends Specification { def "ensureRulesLoaded logs an error if there is an exception loading the rules"() { given: def exception = new RuleLoaderException("Error loading rules", new Exception()) - mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> { throw exception } + mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> { throw exception } when: ruleEngine.runRules(Mock(FhirResource)) @@ -79,14 +78,14 @@ class RuleEngineTest extends Specification { 1 * mockLogger.logError(_ as String, exception) } - def 'runRules handles logging warning correctly'() { + def "runRules handles logging warning correctly"() { given: def ruleViolationMessage = "Rule violation message" def fullRuleViolationMessage = "Rule violation: " + ruleViolationMessage def fhirBundle = Mock(FhirResource) def invalidRule = Mock(Rule) invalidRule.getMessage() >> ruleViolationMessage - mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> [invalidRule] + mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [invalidRule] when: invalidRule.shouldRun(fhirBundle) >> true diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy index 701493bea..68883c1cf 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy @@ -77,7 +77,6 @@ class ValidationRuleTest extends Specification { def "isValid returns expected boolean depending on validations"() { given: def mockFhir = Mock(HapiFhir) - mockFhir.evaluateCondition(_ as Object, _ as String) >> true >> validationResult TestApplicationContext.register(HapiFhir, mockFhir) def rule = new ValidationRule(null, null, null, null, [ @@ -85,13 +84,21 @@ class ValidationRuleTest extends Specification { "secondValidation" ]) - expect: - rule.isValid(new FhirResourceMock("resource")) == valid + when: + mockFhir.evaluateCondition(_ as Object, _ as String) >> true >> true + rule.runRule(new FhirResourceMock("resource")) - where: - validationResult | valid - true | true - false | false + then: + 0 * mockLogger.logWarning(_ as String) + 0 * mockLogger.logError(_ as String, _ as Exception) + + when: + mockFhir.evaluateCondition(_ as Object, _ as String) >> true >> false + rule.runRule(new FhirResourceMock("resource")) + + then: + 1 * mockLogger.logWarning(_ as String) + 0 * mockLogger.logError(_ as String, _ as Exception) } def "isValid logs an error and returns false if an exception happens when evaluating a validation"() { @@ -103,10 +110,10 @@ class ValidationRuleTest extends Specification { def rule = new ValidationRule(null, null, null, null, ["validation"]) when: - def valid = rule.isValid(Mock(FhirResource)) + rule.runRule(Mock(FhirResource)) then: + 0 * mockLogger.logWarning(_ as String) 1 * mockLogger.logError(_ as String, _ as Exception) - !valid } } From d399054b91b2ddfc730e964dbe8a6df4287d927d Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Fri, 26 Apr 2024 14:56:47 -0700 Subject: [PATCH 15/32] Removed transformation engine implementation to move into new PR --- .../etor/ruleengine/TransformationRule.java | 38 ------------------- .../resources/transformation_definitions.json | 15 -------- 2 files changed, 53 deletions(-) delete mode 100644 etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java delete mode 100644 etor/src/main/resources/transformation_definitions.json diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java deleted file mode 100644 index 499f8bddc..000000000 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/TransformationRule.java +++ /dev/null @@ -1,38 +0,0 @@ -package gov.hhs.cdc.trustedintermediary.etor.ruleengine; - -import java.util.List; - -public class TransformationRule implements Rule { - @Override - public String getName() { - return ""; - } - - @Override - public String getDescription() { - return ""; - } - - @Override - public String getMessage() { - return ""; - } - - @Override - public List getConditions() { - return List.of(); - } - - @Override - public List getRules() { - return List.of(); - } - - @Override - public boolean shouldRun(FhirResource resource) { - return false; - } - - @Override - public void runRule(FhirResource resource) {} -} diff --git a/etor/src/main/resources/transformation_definitions.json b/etor/src/main/resources/transformation_definitions.json deleted file mode 100644 index b608d1b2c..000000000 --- a/etor/src/main/resources/transformation_definitions.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "definitions": [ { - "name": "Rule name", - "description": "Rule description", - "message": "Log message", - "conditions": [ ], - "rules": [ - { - "name": "Transformation method name", - "args": { } - } - ] - } - ] -} From 76b7d82e14f57f399d7eddecafb88adb4621215b Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Mon, 29 Apr 2024 12:03:52 -0400 Subject: [PATCH 16/32] Update ValidationRuleTest.groovy Fix method names --- .../etor/ruleengine/ValidationRuleTest.groovy | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy index 68883c1cf..623c6a3cd 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy @@ -38,7 +38,7 @@ class ValidationRuleTest extends Specification { rule.getRules() == validations } - def "appliesTo returns expected boolean depending on conditions"() { + def "shouldRun returns expected boolean depending on conditions"() { given: def mockFhir = Mock(HapiFhir) mockFhir.evaluateCondition(_ as Object, _ as String) >> true >> conditionResult @@ -58,7 +58,7 @@ class ValidationRuleTest extends Specification { false | false } - def "appliesTo logs an error and returns false if an exception happens when evaluating a condition"() { + def "shouldRun logs an error and returns false if an exception happens when evaluating a condition"() { given: def mockFhir = Mock(HapiFhirImplementation) mockFhir.evaluateCondition(_ as Object, "condition") >> { throw new Exception() } @@ -74,7 +74,7 @@ class ValidationRuleTest extends Specification { !applies } - def "isValid returns expected boolean depending on validations"() { + def "runRule returns expected boolean depending on validations"() { given: def mockFhir = Mock(HapiFhir) TestApplicationContext.register(HapiFhir, mockFhir) @@ -101,7 +101,7 @@ class ValidationRuleTest extends Specification { 0 * mockLogger.logError(_ as String, _ as Exception) } - def "isValid logs an error and returns false if an exception happens when evaluating a validation"() { + def "runRule logs an error and returns false if an exception happens when evaluating a validation"() { given: def mockFhir = Mock(HapiFhirImplementation) mockFhir.evaluateCondition(_ as Object, "condition") >> { throw new Exception() } From 27ea75d5ca0324104c729ef199f99750b8871230 Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Mon, 29 Apr 2024 12:04:05 -0400 Subject: [PATCH 17/32] Update ValidationRuleEngineTest.groovy Updated one of the failing tests --- .../etor/ruleengine/ValidationRuleEngineTest.groovy | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index cf20bec19..fe981c160 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy @@ -83,21 +83,23 @@ class ValidationRuleEngineTest extends Specification { def ruleViolationMessage = "Rule violation message" def fullRuleViolationMessage = "Rule violation: " + ruleViolationMessage def fhirBundle = Mock(FhirResource) - def invalidRule = Mock(Rule) + def invalidRule = Mock(ValidationRule) invalidRule.getMessage() >> ruleViolationMessage + invalidRule.shouldRun(fhirBundle) >> true mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [invalidRule] when: - invalidRule.shouldRun(fhirBundle) >> true - invalidRule.isValid(fhirBundle) >> false + invalidRule.runRule(fhirBundle) >> { + mockLogger.logWarning(fullRuleViolationMessage) + } ruleEngine.runRules(fhirBundle) then: 1 * mockLogger.logWarning(fullRuleViolationMessage) when: + invalidRule.runRule(fhirBundle) >> null invalidRule.shouldRun(fhirBundle) >> true - invalidRule.isValid(fhirBundle) >> true ruleEngine.runRules(fhirBundle) then: @@ -105,7 +107,6 @@ class ValidationRuleEngineTest extends Specification { when: invalidRule.shouldRun(fhirBundle) >> false - invalidRule.isValid(fhirBundle) >> false ruleEngine.runRules(fhirBundle) then: From 0f1b27513ba3595715013e2013afcc6f67e3994a Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Mon, 29 Apr 2024 12:46:48 -0400 Subject: [PATCH 18/32] Update ValidationRuleEngine.java Use double checked locking on ensureRulesloaded --- .../etor/ruleengine/ValidationRuleEngine.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index 1f2d77c45..3b59e5994 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -27,11 +27,13 @@ public void unloadRules() { @Override public void ensureRulesLoaded() { - synchronized (this) { - if (rules.isEmpty()) { - List parsedRules = - ruleLoader.loadRules(ruleDefinitionsFileName, new TypeReference<>() {}); - loadRules(parsedRules); + if (rules.isEmpty()) { + synchronized (this) { + if (rules.isEmpty()) { + List parsedRules = + ruleLoader.loadRules(ruleDefinitionsFileName, new TypeReference<>() {}); + loadRules(parsedRules); + } } } } From 5acab7ae321e88651bd05da7ee4cd9590af840f2 Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Mon, 29 Apr 2024 12:47:20 -0400 Subject: [PATCH 19/32] Update ValidationRuleEngineTest.groovy Update more test cases --- .../ruleengine/ValidationRuleEngineTest.groovy | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index fe981c160..0eac4cf5d 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy @@ -37,20 +37,25 @@ class ValidationRuleEngineTest extends Specification { } def "ensureRulesLoaded loads rules only once by default"() { + given: + mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [] + when: ruleEngine.ensureRulesLoaded() ruleEngine.ensureRulesLoaded() // Call twice to test if rules are loaded only once then: - 1 * mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> [Mock(Rule)] + 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) } def "ensureRulesLoaded loads rules only once on multiple threads"() { given: def threadsNum = 10 def iterations = 4 + def mockRule = Mock(ValidationRule) when: + mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] List threads = [] (1..threadsNum).each { threadId -> threads.add(new Thread({ @@ -63,13 +68,16 @@ class ValidationRuleEngineTest extends Specification { threads*.join() then: - 1 * mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> [Mock(Rule)] + 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) } def "ensureRulesLoaded logs an error if there is an exception loading the rules"() { given: def exception = new RuleLoaderException("Error loading rules", new Exception()) - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> { throw exception } + mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> { + mockLogger.logError("Error loading rules", exception) + return [] + } when: ruleEngine.runRules(Mock(FhirResource)) From 28b0d7afc7015441aa609b86e82ee74f8b95e98b Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Mon, 29 Apr 2024 13:04:23 -0400 Subject: [PATCH 20/32] Update ValidationRuleEngineTest.groovy All tests pass --- .../etor/ruleengine/ValidationRuleEngineTest.groovy | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index 0eac4cf5d..b3e84a546 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy @@ -9,6 +9,7 @@ class ValidationRuleEngineTest extends Specification { def ruleEngine = ValidationRuleEngine.getInstance("validation_definitions.json") def mockRuleLoader = Mock(RuleLoader) def mockLogger = Mock(Logger) + def mockRule = Mock(ValidationRule) def setup() { ruleEngine.unloadRules() @@ -24,35 +25,33 @@ class ValidationRuleEngineTest extends Specification { def "ensureRulesLoaded happy path"() { given: - def mockRuleLoader = Mock(RuleLoader) - TestApplicationContext.register(RuleLoader, mockRuleLoader) - TestApplicationContext.injectRegisteredImplementations() + mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] when: ruleEngine.ensureRulesLoaded() then: - 1 * mockRuleLoader.loadRules(_ as String, ValidationRule.class) >> [Mock(Rule)] + 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] ruleEngine.rules.size() == 1 } def "ensureRulesLoaded loads rules only once by default"() { given: - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [] + mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] when: ruleEngine.ensureRulesLoaded() ruleEngine.ensureRulesLoaded() // Call twice to test if rules are loaded only once then: - 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) + 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + ruleEngine.rules.size() == 1 } def "ensureRulesLoaded loads rules only once on multiple threads"() { given: def threadsNum = 10 def iterations = 4 - def mockRule = Mock(ValidationRule) when: mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] From 1f57bb1b02f7c3bb2978abcb6c843eacc3d3b4ef Mon Sep 17 00:00:00 2001 From: saquino0827 Date: Mon, 29 Apr 2024 12:28:03 -0500 Subject: [PATCH 21/32] Update Rule loading to handle Path for flexibility --- .../etor/ruleengine/RuleLoader.java | 10 +++--- .../etor/ruleengine/ValidationRuleEngine.java | 11 +++++- .../etor/ruleengine/RuleLoaderTest.groovy | 35 +++++++++++++------ 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java index 7745dc95c..e5cfa7112 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java @@ -7,6 +7,8 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.Collections; import java.util.List; import java.util.Map; @@ -24,10 +26,8 @@ public static RuleLoader getInstance() { return INSTANCE; } - public List loadRules( - String fileName, TypeReference>> typeReference) { - try (InputStream ruleDefinitionStream = - getClass().getClassLoader().getResourceAsStream(fileName)) { + public List loadRules(Path path, TypeReference>> typeReference) { + try (InputStream ruleDefinitionStream = Files.newInputStream(path)) { assert ruleDefinitionStream != null; var rulesString = new String(ruleDefinitionStream.readAllBytes(), StandardCharsets.UTF_8); @@ -35,7 +35,7 @@ public List loadRules( formatter.convertJsonToObject(rulesString, typeReference); return jsonObj.getOrDefault("definitions", Collections.emptyList()); } catch (IOException | FormatterProcessingException e) { - logger.logError("Failed to load rules definitions from: " + fileName, e); + logger.logError("Failed to load rules definitions from: " + path.getFileName(), e); return Collections.emptyList(); } } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index 1f2d77c45..bd934718c 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -1,6 +1,8 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; import javax.inject.Inject; @@ -29,8 +31,15 @@ public void unloadRules() { public void ensureRulesLoaded() { synchronized (this) { if (rules.isEmpty()) { + Path path = + Paths.get( + getClass() + .getClassLoader() + .getResource(ruleDefinitionsFileName) + .getPath()); + List parsedRules = - ruleLoader.loadRules(ruleDefinitionsFileName, new TypeReference<>() {}); + ruleLoader.loadRules(path, new TypeReference<>() {}); loadRules(parsedRules); } } diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy index ca093b83c..6887b46a0 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy @@ -4,11 +4,16 @@ import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.external.jackson.Jackson import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter +import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference import spock.lang.Specification +import java.nio.file.Files +import java.nio.file.Path + class RuleLoaderTest extends Specification { String fileContents + Path tempFile def setup() { TestApplicationContext.reset() @@ -16,17 +21,25 @@ class RuleLoaderTest extends Specification { TestApplicationContext.register(RuleLoader, RuleLoader.getInstance()) TestApplicationContext.injectRegisteredImplementations() + tempFile = Files.createTempFile("test_validation_definition", ".json") fileContents = """ - { - "rules": [ + { + "definitions": [ { - "name": "patientName", - "conditions": ["Patient.name.exists()"], - "validations": ["Patient.name.where(use='usual').given.exists()"] + "name": "patientName", + "description": "a test rule", + "message": "testing the message", + "conditions": ["Patient.name.exists()"], + "rules": ["Patient.name.where(use='usual').given.exists()"] } - ] - } - """ + ] + } + """ + Files.writeString(tempFile, fileContents) + } + + def cleanup(){ + Files.deleteIfExists(tempFile) } def "load rules from file"() { @@ -36,19 +49,21 @@ class RuleLoaderTest extends Specification { TestApplicationContext.injectRegisteredImplementations() when: - List rules = RuleLoader.getInstance().loadRules(fileContents) + List rules = RuleLoader.getInstance().loadRules(tempFile, new TypeReference>>() {}) then: rules.size() == 1 ValidationRule rule = rules.get(0) as ValidationRule rule.getName() == "patientName" + rule.getDescription() == "a test rule" + rule.getMessage() == "testing the message" rule.getConditions() == ["Patient.name.exists()"] rule.getRules() == [ "Patient.name.where(use='usual').given.exists()" ] } - def "handle FormatterProcessingException when loading rules from file"() { + def "handle FormatterProcessingException when loading rules from a non existent file"() { when: RuleLoader.getInstance().loadRules("!K@WJ#8uhy") From 34a2bf9c40b2d647e6bf80cfaf1f37243bc44219 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 10:57:05 -0700 Subject: [PATCH 22/32] Fixed remaining test --- .../etor/ruleengine/RuleEngine.java | 2 +- .../etor/ruleengine/RuleLoader.java | 8 +++--- .../etor/ruleengine/ValidationRuleEngine.java | 11 ++++++-- .../etor/ruleengine/RuleLoaderTest.groovy | 26 ++++++++++--------- 4 files changed, 28 insertions(+), 19 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 917acfa32..7c4ffb11e 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 @@ -4,7 +4,7 @@ public interface RuleEngine { void unloadRules(); - void ensureRulesLoaded(); + void ensureRulesLoaded() throws RuleLoaderException; void runRules(FhirResource resource); } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java index e5cfa7112..7992a01d6 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoader.java @@ -26,17 +26,17 @@ public static RuleLoader getInstance() { return INSTANCE; } - public List loadRules(Path path, TypeReference>> typeReference) { + public List loadRules(Path path, TypeReference>> typeReference) + throws RuleLoaderException { try (InputStream ruleDefinitionStream = Files.newInputStream(path)) { - assert ruleDefinitionStream != null; var rulesString = new String(ruleDefinitionStream.readAllBytes(), StandardCharsets.UTF_8); Map> jsonObj = formatter.convertJsonToObject(rulesString, typeReference); return jsonObj.getOrDefault("definitions", Collections.emptyList()); } catch (IOException | FormatterProcessingException e) { - logger.logError("Failed to load rules definitions from: " + path.getFileName(), e); - return Collections.emptyList(); + throw new RuleLoaderException( + "Failed to load rules definitions for provided path: " + path, e); } } } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index 474b83ba9..523aa674a 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -1,5 +1,6 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; +import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference; import java.nio.file.Path; import java.nio.file.Paths; @@ -13,6 +14,7 @@ public class ValidationRuleEngine implements RuleEngine { private static final ValidationRuleEngine INSTANCE = new ValidationRuleEngine(); + @Inject Logger logger; @Inject RuleLoader ruleLoader; public static ValidationRuleEngine getInstance(String ruleDefinitionsFileName) { @@ -28,7 +30,7 @@ public void unloadRules() { } @Override - public void ensureRulesLoaded() { + public void ensureRulesLoaded() throws RuleLoaderException { if (rules.isEmpty()) { synchronized (this) { if (rules.isEmpty()) { @@ -49,7 +51,12 @@ public void ensureRulesLoaded() { @Override public void runRules(FhirResource resource) { - ensureRulesLoaded(); + try { + ensureRulesLoaded(); + } catch (RuleLoaderException e) { + logger.logError("Failed to load rules definitions", e); + return; + } for (ValidationRule rule : rules) { if (rule.shouldRun(resource)) { rule.runRule(resource); diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy index 6887b46a0..38f31ab70 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy @@ -19,9 +19,19 @@ class RuleLoaderTest extends Specification { TestApplicationContext.reset() TestApplicationContext.init() TestApplicationContext.register(RuleLoader, RuleLoader.getInstance()) + TestApplicationContext.register(Formatter, Jackson.getInstance()) + TestApplicationContext.register(HapiFhir, Mock(HapiFhir)) TestApplicationContext.injectRegisteredImplementations() tempFile = Files.createTempFile("test_validation_definition", ".json") + } + + def cleanup(){ + Files.deleteIfExists(tempFile) + } + + def "load rules from file"() { + given: fileContents = """ { "definitions": [ @@ -36,17 +46,6 @@ class RuleLoaderTest extends Specification { } """ Files.writeString(tempFile, fileContents) - } - - def cleanup(){ - Files.deleteIfExists(tempFile) - } - - def "load rules from file"() { - given: - TestApplicationContext.register(HapiFhir, Mock(HapiFhir)) - TestApplicationContext.register(Formatter, Jackson.getInstance()) - TestApplicationContext.injectRegisteredImplementations() when: List rules = RuleLoader.getInstance().loadRules(tempFile, new TypeReference>>() {}) @@ -64,8 +63,11 @@ class RuleLoaderTest extends Specification { } def "handle FormatterProcessingException when loading rules from a non existent file"() { + given: + Files.writeString(tempFile, "!K@WJ#8uhy") + when: - RuleLoader.getInstance().loadRules("!K@WJ#8uhy") + RuleLoader.getInstance().loadRules(tempFile, new TypeReference>>() {}) then: thrown(RuleLoaderException) From e137c1dce7a916caf549f8dcbfc74b4a814cab3d Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 11:12:34 -0700 Subject: [PATCH 23/32] Fixed actual remaining test --- .../ValidationRuleEngineTest.groovy | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index b3e84a546..5423d4e85 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy @@ -5,6 +5,8 @@ import gov.hhs.cdc.trustedintermediary.wrappers.Logger import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference import spock.lang.Specification +import java.nio.file.Path + class ValidationRuleEngineTest extends Specification { def ruleEngine = ValidationRuleEngine.getInstance("validation_definitions.json") def mockRuleLoader = Mock(RuleLoader) @@ -25,26 +27,26 @@ class ValidationRuleEngineTest extends Specification { def "ensureRulesLoaded happy path"() { given: - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] when: ruleEngine.ensureRulesLoaded() then: - 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + 1 * mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] ruleEngine.rules.size() == 1 } def "ensureRulesLoaded loads rules only once by default"() { given: - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] when: ruleEngine.ensureRulesLoaded() ruleEngine.ensureRulesLoaded() // Call twice to test if rules are loaded only once then: - 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + 1 * mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] ruleEngine.rules.size() == 1 } @@ -54,7 +56,7 @@ class ValidationRuleEngineTest extends Specification { def iterations = 4 when: - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] List threads = [] (1..threadsNum).each { threadId -> threads.add(new Thread({ @@ -67,13 +69,13 @@ class ValidationRuleEngineTest extends Specification { threads*.join() then: - 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) + 1 * mockRuleLoader.loadRules(_ as Path, _ as TypeReference) } def "ensureRulesLoaded logs an error if there is an exception loading the rules"() { given: def exception = new RuleLoaderException("Error loading rules", new Exception()) - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> { + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> { mockLogger.logError("Error loading rules", exception) return [] } @@ -88,12 +90,12 @@ class ValidationRuleEngineTest extends Specification { def "runRules handles logging warning correctly"() { given: def ruleViolationMessage = "Rule violation message" - def fullRuleViolationMessage = "Rule violation: " + ruleViolationMessage + def fullRuleViolationMessage = "Validation failed: " + ruleViolationMessage def fhirBundle = Mock(FhirResource) def invalidRule = Mock(ValidationRule) invalidRule.getMessage() >> ruleViolationMessage invalidRule.shouldRun(fhirBundle) >> true - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [invalidRule] + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [invalidRule] when: invalidRule.runRule(fhirBundle) >> { From 0fc78c570658f179b7c268c4c6a80c72482f604f Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 11:15:38 -0700 Subject: [PATCH 24/32] Changed naming --- .../etor/ruleengine/ValidationRule.java | 2 +- .../ruleengine/ValidationRuleEngineTest.groovy | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java index 74870b52f..f25db62f8 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java @@ -92,7 +92,7 @@ public void runRule(FhirResource resource) { boolean isValid = fhirEngine.evaluateCondition(resource.getUnderlyingResource(), validation); if (!isValid) { - logger.logWarning("Rule violation: " + message); + logger.logWarning("Validation failed: " + message); } } catch (Exception e) { logger.logError( diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index 5423d4e85..0a6e38a5f 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy @@ -89,22 +89,22 @@ class ValidationRuleEngineTest extends Specification { def "runRules handles logging warning correctly"() { given: - def ruleViolationMessage = "Rule violation message" - def fullRuleViolationMessage = "Validation failed: " + ruleViolationMessage + def failedValidationMessage = "Failed validation message" + def fullFailedValidationMessage = "Validation failed: " + failedValidationMessage def fhirBundle = Mock(FhirResource) def invalidRule = Mock(ValidationRule) - invalidRule.getMessage() >> ruleViolationMessage + invalidRule.getMessage() >> failedValidationMessage invalidRule.shouldRun(fhirBundle) >> true mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [invalidRule] when: invalidRule.runRule(fhirBundle) >> { - mockLogger.logWarning(fullRuleViolationMessage) + mockLogger.logWarning(fullFailedValidationMessage) } ruleEngine.runRules(fhirBundle) then: - 1 * mockLogger.logWarning(fullRuleViolationMessage) + 1 * mockLogger.logWarning(fullFailedValidationMessage) when: invalidRule.runRule(fhirBundle) >> null @@ -112,13 +112,13 @@ class ValidationRuleEngineTest extends Specification { ruleEngine.runRules(fhirBundle) then: - 0 * mockLogger.logWarning(fullRuleViolationMessage) + 0 * mockLogger.logWarning(fullFailedValidationMessage) when: invalidRule.shouldRun(fhirBundle) >> false ruleEngine.runRules(fhirBundle) then: - 0 * mockLogger.logWarning(fullRuleViolationMessage) + 0 * mockLogger.logWarning(fullFailedValidationMessage) } } From 95d4b2dee1aeaf5e94ad2001b0de6f559b5df39e Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Mon, 29 Apr 2024 14:16:22 -0400 Subject: [PATCH 25/32] Update ValidationRuleEngineTest.groovy Update tests to work with refactoring changes --- .../ruleengine/ValidationRuleEngineTest.groovy | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index b3e84a546..321d2c59c 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy @@ -5,6 +5,8 @@ import gov.hhs.cdc.trustedintermediary.wrappers.Logger import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference import spock.lang.Specification +import java.nio.file.Path + class ValidationRuleEngineTest extends Specification { def ruleEngine = ValidationRuleEngine.getInstance("validation_definitions.json") def mockRuleLoader = Mock(RuleLoader) @@ -25,26 +27,26 @@ class ValidationRuleEngineTest extends Specification { def "ensureRulesLoaded happy path"() { given: - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] when: ruleEngine.ensureRulesLoaded() then: - 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + 1 * mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] ruleEngine.rules.size() == 1 } def "ensureRulesLoaded loads rules only once by default"() { given: - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] when: ruleEngine.ensureRulesLoaded() ruleEngine.ensureRulesLoaded() // Call twice to test if rules are loaded only once then: - 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + 1 * mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] ruleEngine.rules.size() == 1 } @@ -54,7 +56,7 @@ class ValidationRuleEngineTest extends Specification { def iterations = 4 when: - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [mockRule] + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [mockRule] List threads = [] (1..threadsNum).each { threadId -> threads.add(new Thread({ @@ -67,13 +69,13 @@ class ValidationRuleEngineTest extends Specification { threads*.join() then: - 1 * mockRuleLoader.loadRules(_ as String, _ as TypeReference) + 1 * mockRuleLoader.loadRules(_ as Path, _ as TypeReference) } def "ensureRulesLoaded logs an error if there is an exception loading the rules"() { given: def exception = new RuleLoaderException("Error loading rules", new Exception()) - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> { + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> { mockLogger.logError("Error loading rules", exception) return [] } @@ -93,7 +95,7 @@ class ValidationRuleEngineTest extends Specification { def invalidRule = Mock(ValidationRule) invalidRule.getMessage() >> ruleViolationMessage invalidRule.shouldRun(fhirBundle) >> true - mockRuleLoader.loadRules(_ as String, _ as TypeReference) >> [invalidRule] + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> [invalidRule] when: invalidRule.runRule(fhirBundle) >> { From c0f0093a1ce6fe3e797bbd6974e7d07175c597ed Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 11:24:35 -0700 Subject: [PATCH 26/32] Added test coverage --- .../etor/ruleengine/ValidationRuleEngineTest.groovy | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index 0a6e38a5f..88bb90e57 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy @@ -121,4 +121,17 @@ class ValidationRuleEngineTest extends Specification { then: 0 * mockLogger.logWarning(fullFailedValidationMessage) } + + + def "runRules logs an error and doesn't run any rules when there's a RuleLoaderException"() { + given: + def exception = new RuleLoaderException("Error loading rules", new Exception()) + mockRuleLoader.loadRules(_ as Path, _ as TypeReference) >> { throw exception } + + when: + ruleEngine.runRules(Mock(FhirResource)) + + then: + 1 * mockLogger.logError(_ as String, exception) + } } From 2a5f7abb5c59ce24d54cf30bc61905d2bed1ccbc Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 11:31:20 -0700 Subject: [PATCH 27/32] Updated javadocs --- .../cdc/trustedintermediary/etor/ruleengine/RuleEngine.java | 5 ++++- .../etor/ruleengine/ValidationRuleEngine.java | 1 + 2 files changed, 5 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 7c4ffb11e..6357598d8 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 @@ -1,6 +1,9 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; -/** Manages the application of rules loaded from a definitions file using the RuleLoader. */ +/** + * The RuleEngine interface defines the structure for a rule engine. Each rule engine has methods to + * load rules, ensure rules are loaded, and run rules on a resource. + */ public interface RuleEngine { void unloadRules(); diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java index 523aa674a..e409af3da 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java @@ -8,6 +8,7 @@ import java.util.List; import javax.inject.Inject; +/** Implements the RuleEngine interface. It represents a rule engine for validations. */ public class ValidationRuleEngine implements RuleEngine { private String ruleDefinitionsFileName; final List rules = new ArrayList<>(); From 4a5068db11f3a2c9cb1e9c25b3df468f6c8ca0a8 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 12:58:15 -0700 Subject: [PATCH 28/32] Created trasnformation package and moved transformation engine files --- .../trustedintermediary/etor/EtorDomainRegistration.java | 2 +- .../trustedintermediary/etor/orders/OrderController.java | 2 +- .../etor/ruleengine/{ => validation}/ValidationRule.java | 4 +++- .../ruleengine/{ => validation}/ValidationRuleEngine.java | 6 +++++- .../etor/orders/OrderControllerTest.groovy | 2 +- .../etor/ruleengine/RuleLoaderTest.groovy | 1 + .../ruleengine/ValidationRuleEngineIntegrationTest.groovy | 2 ++ .../etor/ruleengine/ValidationRuleEngineTest.groovy | 2 ++ .../etor/ruleengine/ValidationRuleTest.groovy | 1 + 9 files changed, 17 insertions(+), 5 deletions(-) rename etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/{ => validation}/ValidationRule.java (94%) rename etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/{ => validation}/ValidationRuleEngine.java (86%) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java index d839a91e3..bad7fbabc 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/EtorDomainRegistration.java @@ -34,7 +34,7 @@ import gov.hhs.cdc.trustedintermediary.etor.results.ResultSender; import gov.hhs.cdc.trustedintermediary.etor.results.SendResultUseCase; import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleLoader; -import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRuleEngine; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRuleEngine; import gov.hhs.cdc.trustedintermediary.external.database.DatabaseMessageLinkStorage; import gov.hhs.cdc.trustedintermediary.external.database.DatabasePartnerMetadataStorage; import gov.hhs.cdc.trustedintermediary.external.database.DbDao; diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java index 4bd37e334..a841c29fd 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/orders/OrderController.java @@ -2,7 +2,7 @@ import gov.hhs.cdc.trustedintermediary.domainconnector.DomainRequest; import gov.hhs.cdc.trustedintermediary.etor.metadata.EtorMetadataStep; -import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRuleEngine; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRuleEngine; import gov.hhs.cdc.trustedintermediary.external.hapi.HapiFhirResource; import gov.hhs.cdc.trustedintermediary.external.hapi.HapiOrder; import gov.hhs.cdc.trustedintermediary.wrappers.FhirParseException; diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java similarity index 94% rename from etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java rename to etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java index f25db62f8..dfb0a42a1 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java @@ -1,6 +1,8 @@ -package gov.hhs.cdc.trustedintermediary.etor.ruleengine; +package gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation; import gov.hhs.cdc.trustedintermediary.context.ApplicationContext; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.FhirResource; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.Rule; import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir; import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import java.util.List; diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java similarity index 86% rename from etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java rename to etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java index e409af3da..fb1e1adc0 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java @@ -1,5 +1,9 @@ -package gov.hhs.cdc.trustedintermediary.etor.ruleengine; +package gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.FhirResource; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleEngine; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleLoader; +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.RuleLoaderException; import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference; import java.nio.file.Path; diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy index 034072f68..f1f8c794c 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/orders/OrderControllerTest.groovy @@ -3,7 +3,7 @@ package gov.hhs.cdc.trustedintermediary.etor.orders import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext import gov.hhs.cdc.trustedintermediary.domainconnector.DomainRequest import gov.hhs.cdc.trustedintermediary.etor.metadata.EtorMetadataStep -import gov.hhs.cdc.trustedintermediary.etor.ruleengine.ValidationRuleEngine +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRuleEngine import gov.hhs.cdc.trustedintermediary.external.hapi.HapiMessageHelper import gov.hhs.cdc.trustedintermediary.wrappers.FhirParseException import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy index 38f31ab70..85edab9fe 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleLoaderTest.groovy @@ -1,6 +1,7 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRule import gov.hhs.cdc.trustedintermediary.external.jackson.Jackson import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir import gov.hhs.cdc.trustedintermediary.wrappers.formatter.Formatter diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy index 3cef1191e..98903751c 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy @@ -1,6 +1,8 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRule +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRuleEngine import gov.hhs.cdc.trustedintermediary.external.hapi.HapiFhirImplementation import gov.hhs.cdc.trustedintermediary.external.hapi.HapiFhirResource import gov.hhs.cdc.trustedintermediary.external.jackson.Jackson diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy index 88bb90e57..e52261f2d 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineTest.groovy @@ -1,6 +1,8 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRule +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRuleEngine import gov.hhs.cdc.trustedintermediary.wrappers.Logger import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference import spock.lang.Specification diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy index 623c6a3cd..b6fb44ad6 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleTest.groovy @@ -2,6 +2,7 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine import gov.hhs.cdc.trustedintermediary.FhirResourceMock import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext +import gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation.ValidationRule import gov.hhs.cdc.trustedintermediary.external.hapi.HapiFhirImplementation import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir import gov.hhs.cdc.trustedintermediary.wrappers.Logger From 2bd21c8e5ce94cdc994fa525e70650d27fd70b34 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 15:30:21 -0700 Subject: [PATCH 29/32] Changed implementation for rules implementation to inherit from Rule class --- .../etor/ruleengine/Rule.java | 83 +++++++++++++++--- .../ruleengine/validation/ValidationRule.java | 84 +++---------------- ...ValidationRuleEngineIntegrationTest.groovy | 14 ++-- 3 files changed, 91 insertions(+), 90 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java index e7d27365f..1b421cfed 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/Rule.java @@ -1,24 +1,85 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine; +import gov.hhs.cdc.trustedintermediary.context.ApplicationContext; +import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir; +import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import java.util.List; /** - * The Rule interface defines the structure for a rule in the rule engine. Each rule has a name, - * description, warning message, conditions, validations, and methods to check if a resource is - * valid and if the rule applies to a resource. + * Represents a rule that can be run on a FHIR resource. Each rule has a name, description, logging + * message, conditions to determine if the rule should run, and actions to run in case the condition + * is met. */ -public interface Rule { - String getName(); +public class Rule { - String getDescription(); + protected final Logger logger = ApplicationContext.getImplementation(Logger.class); + protected final HapiFhir fhirEngine = ApplicationContext.getImplementation(HapiFhir.class); + private String name; + private String description; + private String message; + private List conditions; + private List rules; - String getMessage(); + /** + * Do not delete this constructor! It is used for JSON deserialization when loading rules from a + * file. + */ + public Rule() {} - List getConditions(); + public Rule( + String ruleName, + String ruleDescription, + String ruleMessage, + List ruleConditions, + List ruleActions) { + name = ruleName; + description = ruleDescription; + message = ruleMessage; + conditions = ruleConditions; + rules = ruleActions; + } - List getRules(); + public String getName() { + return name; + } - boolean shouldRun(FhirResource resource); + public String getDescription() { + return description; + } - void runRule(FhirResource resource); + public String getMessage() { + return message; + } + + public List getConditions() { + return conditions; + } + + public List getRules() { + return rules; + } + + public boolean shouldRun(FhirResource resource) { + return conditions.stream() + .allMatch( + condition -> { + try { + return fhirEngine.evaluateCondition( + resource.getUnderlyingResource(), condition); + } catch (Exception e) { + logger.logError( + "Rule [" + + name + + "]: " + + "An error occurred while evaluating the condition: " + + condition, + e); + return false; + } + }); + } + + public void runRule(FhirResource resource) { + throw new UnsupportedOperationException("This method must be implemented by subclasses."); + } } diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java index dfb0a42a1..abaacfa42 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java @@ -1,26 +1,16 @@ package gov.hhs.cdc.trustedintermediary.etor.ruleengine.validation; -import gov.hhs.cdc.trustedintermediary.context.ApplicationContext; import gov.hhs.cdc.trustedintermediary.etor.ruleengine.FhirResource; import gov.hhs.cdc.trustedintermediary.etor.ruleengine.Rule; -import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir; -import gov.hhs.cdc.trustedintermediary.wrappers.Logger; import java.util.List; /** - * Implements the Rule interface. It represents a rule with a name, description, warning message, - * conditions, and validations. It uses the HapiFhir engine to evaluate the conditions and - * validations. + * The ValidationRule class extends the {@link gov.hhs.cdc.trustedintermediary.etor.ruleengine.Rule + * Rule} class and represents a validation rule. It implements the {@link + * gov.hhs.cdc.trustedintermediary.etor.ruleengine.Rule#runRule(FhirResource) runRule} method to + * evaluate the validation and log a warning if the validation fails. */ -public class ValidationRule implements Rule { - - private final Logger logger = ApplicationContext.getImplementation(Logger.class); - private final HapiFhir fhirEngine = ApplicationContext.getImplementation(HapiFhir.class); - private String name; - private String description; - private String message; - private List conditions; - private List rules; +public class ValidationRule extends Rule { /** * Do not delete this constructor! It is used for JSON deserialization when loading rules from a @@ -34,72 +24,22 @@ public ValidationRule( String ruleMessage, List ruleConditions, List ruleActions) { - name = ruleName; - description = ruleDescription; - message = ruleMessage; - conditions = ruleConditions; - rules = ruleActions; - } - - @Override - public String getName() { - return name; - } - - @Override - public String getDescription() { - return description; - } - - @Override - public String getMessage() { - return message; - } - - @Override - public List getConditions() { - return conditions; - } - - @Override - public List getRules() { - return rules; - } - - @Override - public boolean shouldRun(FhirResource resource) { - return conditions.stream() - .allMatch( - condition -> { - try { - return fhirEngine.evaluateCondition( - resource.getUnderlyingResource(), condition); - } catch (Exception e) { - logger.logError( - "Rule [" - + name - + "]: " - + "An error occurred while evaluating the condition: " - + condition, - e); - return false; - } - }); + super(ruleName, ruleDescription, ruleMessage, ruleConditions, ruleActions); } - @Override public void runRule(FhirResource resource) { - for (String validation : rules) { + for (String validation : this.getRules()) { try { boolean isValid = - fhirEngine.evaluateCondition(resource.getUnderlyingResource(), validation); + this.fhirEngine.evaluateCondition( + resource.getUnderlyingResource(), validation); if (!isValid) { - logger.logWarning("Validation failed: " + message); + this.logger.logWarning("Validation failed: " + this.getMessage()); } } catch (Exception e) { - logger.logError( + this.logger.logError( "Rule [" - + name + + this.getName() + "]: " + "An error occurred while evaluating the validation: " + validation, diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy index 98903751c..001ce7bff 100644 --- a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/ValidationRuleEngineIntegrationTest.groovy @@ -66,7 +66,7 @@ class ValidationRuleEngineIntegrationTest extends Specification { given: def fhirResource = getExampleFhirResource("e2e/orders/001_OML_O21_short.fhir") def validation = "Bundle.entry.resource.ofType(MessageHeader).focus.resolve().category.exists()" - Rule rule = createValidationRule([], [validation]) + def rule = createValidationRule([], [validation]) when: rule.runRule(fhirResource) @@ -184,13 +184,13 @@ class ValidationRuleEngineIntegrationTest extends Specification { return bundle } - Rule createValidationRule(List ruleConditions, List ruleValidations) { + ValidationRule createValidationRule(List ruleConditions, List ruleValidations) { return new ValidationRule( - name: "Rule name", - description: "Rule description", - message: "Rule warning message", - conditions: ruleConditions, - rules: ruleValidations, + "Rule name", + "Rule description", + "Rule warning message", + ruleConditions, + ruleValidations, ) } From 6ba19436264d0d2515cd6f69e5589959705f2d84 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 15:30:39 -0700 Subject: [PATCH 30/32] Clean up --- .../etor/ruleengine/validation/ValidationRuleEngine.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java index fb1e1adc0..8554a6394 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRuleEngine.java @@ -48,7 +48,7 @@ public void ensureRulesLoaded() throws RuleLoaderException { List parsedRules = ruleLoader.loadRules(path, new TypeReference<>() {}); - loadRules(parsedRules); + this.rules.addAll(parsedRules); } } } @@ -68,8 +68,4 @@ public void runRules(FhirResource resource) { } } } - - private synchronized void loadRules(List rules) { - this.rules.addAll(rules); - } } From a03dd0b0eeed3c4371e828e7fe872fe0b1dba851 Mon Sep 17 00:00:00 2001 From: Basilio Bogado <541149+basiliskus@users.noreply.github.com> Date: Mon, 29 Apr 2024 16:28:28 -0700 Subject: [PATCH 31/32] Added missing override tag --- .../etor/ruleengine/validation/ValidationRule.java | 1 + 1 file changed, 1 insertion(+) diff --git a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java index abaacfa42..4dbcdc57f 100644 --- a/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java +++ b/etor/src/main/java/gov/hhs/cdc/trustedintermediary/etor/ruleengine/validation/ValidationRule.java @@ -27,6 +27,7 @@ public ValidationRule( super(ruleName, ruleDescription, ruleMessage, ruleConditions, ruleActions); } + @Override public void runRule(FhirResource resource) { for (String validation : this.getRules()) { try { From e9290aa4f47e0b2df2d0e162b2538c62f12d68f5 Mon Sep 17 00:00:00 2001 From: Luis Pabon Date: Mon, 29 Apr 2024 20:07:30 -0400 Subject: [PATCH 32/32] Create RuleTest.groovy Added test-case for base Rule class exclusive implementation. --- .../etor/ruleengine/RuleTest.groovy | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleTest.groovy diff --git a/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleTest.groovy b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleTest.groovy new file mode 100644 index 000000000..3f335d233 --- /dev/null +++ b/etor/src/test/groovy/gov/hhs/cdc/trustedintermediary/etor/ruleengine/RuleTest.groovy @@ -0,0 +1,29 @@ +package gov.hhs.cdc.trustedintermediary.etor.ruleengine + +import gov.hhs.cdc.trustedintermediary.FhirResourceMock +import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext +import gov.hhs.cdc.trustedintermediary.wrappers.HapiFhir +import gov.hhs.cdc.trustedintermediary.wrappers.Logger +import spock.lang.Specification + +class RuleTest extends Specification { + + def setup() { + TestApplicationContext.reset() + TestApplicationContext.init() + TestApplicationContext.register(Logger, Mock(Logger)) + TestApplicationContext.register(HapiFhir, Mock(HapiFhir)) + TestApplicationContext.injectRegisteredImplementations() + } + + def "runRule throws an UnsupportedOperationException when ran from the Rule class"() { + given: + def rule = new Rule() + + when: + rule.runRule(new FhirResourceMock("resource")) + + then: + thrown(UnsupportedOperationException) + } +}