Skip to content

Commit

Permalink
Rules engine refactoring (#1037)
Browse files Browse the repository at this point in the history
* Refactored validation rule engine in preparation for tranformation engine implementation

* Fixed instantiation

* Refactored to make Rule interface more generic

* Fixed tests

* Update transformation_definitions.json

Added dummy definitions from design discussion

* Added missing override tags

* Fixed arg naming

* Use generics to deserialize correct type of Rule implementation

* Converted RuleEngine back into a singleton and added parameters

* Refactored to have a separate implementation for ValidationRuleEngine

* Refactored to pass TypeReference and fix mapping issue

* Added missing override tags

* Fixed tests in ValidationRuleEngineIntegrationTest

* Fixed more tests

* Removed transformation engine implementation to move into new PR

* Update ValidationRuleTest.groovy

Fix method names

* Update ValidationRuleEngineTest.groovy

Updated one of the failing tests

* Update ValidationRuleEngine.java

Use double checked locking on ensureRulesloaded

* Update ValidationRuleEngineTest.groovy

Update more test cases

* Update ValidationRuleEngineTest.groovy

All tests pass

* Update Rule loading to handle Path for flexibility

* Fixed remaining test

* Fixed actual remaining test

* Changed naming

* Update ValidationRuleEngineTest.groovy

Update tests to work with refactoring changes

* Added test coverage

* Updated javadocs

* Created trasnformation package and moved transformation engine files

* Changed implementation for rules implementation to inherit from Rule class

* Clean up

* Added missing override tag

* Create RuleTest.groovy

Added test-case for base Rule class exclusive implementation.

---------

Co-authored-by: Luis Pabon <[email protected]>
Co-authored-by: saquino0827 <[email protected]>
  • Loading branch information
3 people authored May 3, 2024
1 parent cd52516 commit 32abaaf
Show file tree
Hide file tree
Showing 16 changed files with 490 additions and 362 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.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;
Expand Down Expand Up @@ -133,7 +133,9 @@ public Map<HttpEndpoint, Function<DomainRequest, DomainResponse>> 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("validation_definitions.json"));

ApplicationContext.register(SendMessageHelper.class, SendMessageHelper.getInstance());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.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;
Expand All @@ -20,7 +20,7 @@ public class OrderController {
@Inject HapiFhir fhir;
@Inject Logger logger;
@Inject MetricMetadata metadata;
@Inject RuleEngine ruleEngine;
@Inject ValidationRuleEngine validationEngine;

private OrderController() {}

Expand All @@ -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));
validationEngine.runRules(new HapiFhirResource(fhirBundle));
metadata.put(fhirBundle.getId(), EtorMetadataStep.RECEIVED_FROM_REPORT_STREAM);
return new HapiOrder(fhirBundle);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +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<String> conditions;
private List<String> rules;

/**
* 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!
* Do not delete this constructor! It is used for JSON deserialization when loading rules from a
* file.
*/
String getViolationMessage();
public Rule() {}

List<String> getConditions();
public Rule(
String ruleName,
String ruleDescription,
String ruleMessage,
List<String> ruleConditions,
List<String> ruleActions) {
name = ruleName;
description = ruleDescription;
message = ruleMessage;
conditions = ruleConditions;
rules = ruleActions;
}

List<String> getValidations();
public String getName() {
return name;
}

boolean isValid(FhirResource<?> resource);
public String getDescription() {
return description;
}

boolean appliesTo(FhirResource<?> resource);
public String getMessage() {
return message;
}

public List<String> getConditions() {
return conditions;
}

public List<String> 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.");
}
}
Original file line number Diff line number Diff line change
@@ -1,63 +1,13 @@
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;
/**
* 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();

/** Manages the application of rules loaded from a definitions file using the RuleLoader. */
public class RuleEngine {
void ensureRulesLoaded() throws RuleLoaderException;

private static final RuleEngine INSTANCE = new RuleEngine();

final List<Rule> rules = new ArrayList<>();

@Inject Logger logger;
@Inject RuleLoader ruleLoader;

private RuleEngine() {}

public static RuleEngine getInstance() {
return INSTANCE;
}

public void unloadRules() {
rules.clear();
}

public void ensureRulesLoaded() {
if (rules.isEmpty()) {
synchronized (this) {
if (rules.isEmpty()) {
loadRules();
}
}
}
}

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);
}
}

public void validate(FhirResource<?> resource) {
logger.logDebug("Validating FHIR resource");
ensureRulesLoaded();
for (Rule rule : rules) {
if (rule.appliesTo(resource) && !rule.isValid(resource)) {
logger.logWarning("Rule violation: " + rule.getViolationMessage());
}
}
}
void runRules(FhirResource<?> resource);
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
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.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand All @@ -12,21 +18,25 @@
public class RuleLoader {
private static final RuleLoader INSTANCE = new RuleLoader();
@Inject Formatter formatter;
@Inject Logger logger;

private RuleLoader() {}

public static RuleLoader getInstance() {
return INSTANCE;
}

public List<ValidationRule> loadRules(String ruleStream) throws RuleLoaderException {
try {
Map<String, List<ValidationRule>> jsonObj =
formatter.convertJsonToObject(ruleStream, new TypeReference<>() {});
return jsonObj.getOrDefault("rules", Collections.emptyList());
} catch (FormatterProcessingException e) {
public <T> List<T> loadRules(Path path, TypeReference<Map<String, List<T>>> typeReference)
throws RuleLoaderException {
try (InputStream ruleDefinitionStream = Files.newInputStream(path)) {
var rulesString =
new String(ruleDefinitionStream.readAllBytes(), StandardCharsets.UTF_8);
Map<String, List<T>> jsonObj =
formatter.convertJsonToObject(rulesString, typeReference);
return jsonObj.getOrDefault("definitions", Collections.emptyList());
} catch (IOException | FormatterProcessingException e) {
throw new RuleLoaderException(
"Failed to load rules definitions for provided stream", e);
"Failed to load rules definitions for provided path: " + path, e);
}
}
}

This file was deleted.

Loading

0 comments on commit 32abaaf

Please sign in to comment.