Skip to content

Commit

Permalink
Make sure all assertion rules run at least once (#1586)
Browse files Browse the repository at this point in the history
* Bubble up exceptions, add context, and let test fail

* Throw exception when no matching files found and simplify logic

* Fixed failing test

* Added HapiHL7FileMatcherException custom exception and fixed tests

* Added javadocs

* WIP: Added AzureBlobOrganizer to reorganize blobs by dated folders

* Added cleanup logic, fixed storage paths and misc cleanup

* Refactored AzureBlobOrganizer to better handle cooying and deleting blobs, imrpoved time zone handling, added logging, more accurately check if blob was moved

* Removed non existing metadata and improved how we're checking the blob got copied

* Renaming and cleanup

* Extracted time zone to use the same one everywhere

* Moved methods to helper class

* Changed azure fetch logic to get blobs in expected path for todays date

* Added tests for AzureBlobHelper

* Renamed sourcePath => sourceName

* Added javadocs

* Added assertion to ensure all rules ran

* Added test coverage

* Removed UTC timezone comment. Planning to keep using UTC

* Added comment explaining date path formatting

* Added note about blob retention policy

* Added more context on file org

* Renamed and added comment for clarity
  • Loading branch information
basiliskus authored Nov 15, 2024
1 parent 31438cd commit 7ba2110
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import javax.inject.Inject;

/**
Expand Down Expand Up @@ -58,21 +60,28 @@ public void ensureRulesLoaded() throws RuleLoaderException {
}
}

public void runRules(Message outputMessage, Message inputMessage) {
public List<AssertionRule> getRules() {
return assertionRules;
}

public Set<AssertionRule> runRules(Message outputMessage, Message inputMessage) {
try {
ensureRulesLoaded();
} catch (RuleLoaderException e) {
logger.logError("Failed to load rules definitions", e);
return;
return Set.of();
}

HapiHL7Message outputHapiMessage = new HapiHL7Message(outputMessage);
HapiHL7Message inputHapiMessage = new HapiHL7Message(inputMessage);

Set<AssertionRule> runRules = new HashSet<>();
for (AssertionRule rule : assertionRules) {
if (rule.shouldRun(outputHapiMessage)) {
rule.runRule(outputHapiMessage, inputHapiMessage);
runRules.add(rule);
}
}
return runRules;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ class AutomatedTest extends Specification {

FileFetcher localFileFetcher = LocalFileFetcher.getInstance()
localFiles = localFileFetcher.fetchFiles()

engine.ensureRulesLoaded()
}

def cleanup() {
Expand All @@ -50,16 +52,19 @@ class AutomatedTest extends Specification {

def "test defined assertions on relevant messages"() {
given:
def rulesToEvaluate = engine.getRules()
def matchedFiles = fileMatcher.matchFiles(azureFiles, localFiles)

when:
for (messagePair in matchedFiles) {
Message inputMessage = messagePair.getKey() as Message
Message outputMessage = messagePair.getValue() as Message
engine.runRules(outputMessage, inputMessage)
def evaluatedRules = engine.runRules(outputMessage, inputMessage)
rulesToEvaluate.removeAll(evaluatedRules)
}

then:
rulesToEvaluate.collect { it.name }.isEmpty() //Check whether all the rules in the assertions file have been run
0 * mockLogger.logError(_ as String, _ as Exception)
0 * mockLogger.logWarning(_ as String)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gov.hhs.cdc.trustedintermediary.rse2e.ruleengine

import ca.uhn.hl7v2.model.Message
import gov.hhs.cdc.trustedintermediary.rse2e.external.hapi.HapiHL7Message
import gov.hhs.cdc.trustedintermediary.ruleengine.RuleLoader
import gov.hhs.cdc.trustedintermediary.ruleengine.RuleLoaderException
import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext
Expand Down Expand Up @@ -37,7 +38,7 @@ class AssertionRuleEngineTest extends Specification {

then:
1 * mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [mockRule]
ruleEngine.assertionRules.size() == 1
ruleEngine.getRules().size() == 1
}

def "ensureRulesLoaded loads rules only once by default"() {
Expand All @@ -50,7 +51,7 @@ class AssertionRuleEngineTest extends Specification {

then:
1 * mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [mockRule]
ruleEngine.assertionRules.size() == 1
ruleEngine.getRules().size() == 1
}

def "ensureRulesLoaded loads rules only once on multiple threads"() {
Expand Down Expand Up @@ -101,4 +102,37 @@ class AssertionRuleEngineTest extends Specification {
then:
1 * mockLogger.logError(_ as String, exception)
}

def "runRules returns nothing when there are no rules"() {
when:
def result = ruleEngine.runRules(Mock(Message), Mock(Message))

then:
result.isEmpty()
}

def "runRules returns rules that have been run"() {
given:
def rule = new AssertionRule("testRule", [], [])
mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [rule]

when:
def result = ruleEngine.runRules(Mock(Message), Mock(Message))

then:
result.size() == 1
result[0] == rule
}

def "runRules doesn't return rules that shouldn't run"() {
given:
def rule = new AssertionRule("testRule", ["false"], [])
mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [rule]

when:
def result = ruleEngine.runRules(Mock(Message), Mock(Message))

then:
result.isEmpty()
}
}

0 comments on commit 7ba2110

Please sign in to comment.