Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor HapiHL7Message.getName to return more useful identifier #1636

Merged
merged 6 commits into from
Dec 3, 2024
Merged

Refactor HapiHL7Message.getName to return more useful identifier #1636

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d01ed02
WIP: Refactored to avoid leaking external dependency + some cleanup
basiliskus Dec 2, 2024
dc394ff
Renamed HapiHL7Message.getName => getIdentifier
basiliskus Dec 2, 2024
800c827
Fixed failing tests
basiliskus Dec 3, 2024
45f810d
Fixed flipped parameters
basiliskus Dec 3, 2024
7ffe348
Missed updating Message => HapiHL7Message in HapiHL7FileMatcherTest
basiliskus Dec 3, 2024
d5392a5
Cleanup
basiliskus Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions ...src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import ca.uhn.hl7v2.HL7Exception;
import ca.uhn.hl7v2.HapiContext;
import ca.uhn.hl7v2.model.Message;
import ca.uhn.hl7v2.model.v251.segment.MSH;
import ca.uhn.hl7v2.parser.Parser;
import com.google.common.collect.Sets;
import gov.hhs.cdc.trustedintermediary.rse2e.HL7FileStream;
Expand Down Expand Up @@ -32,13 +31,13 @@ public static HapiHL7FileMatcher getInstance() {
return INSTANCE;
}

public Map<Message, Message> matchFiles(
public Map<HapiHL7Message, HapiHL7Message> matchFiles(
List<HL7FileStream> outputFiles, List<HL7FileStream> inputFiles)
throws HapiHL7FileMatcherException {
// We pair up output and input files based on the control ID, which is in MSH-10
// Any files (either input or output) that don't have a match are logged
Map<String, Message> inputMap = mapMessageByControlId(inputFiles);
Map<String, Message> outputMap = mapMessageByControlId(outputFiles);
Map<String, HapiHL7Message> inputMap = parseAndMapMessageByControlId(inputFiles);
Map<String, HapiHL7Message> outputMap = parseAndMapMessageByControlId(outputFiles);

Set<String> inputKeys = inputMap.keySet();
Set<String> outputKeys = outputMap.keySet();
Expand All @@ -55,10 +54,10 @@ public Map<Message, Message> matchFiles(
return inputKeys.stream().collect(Collectors.toMap(inputMap::get, outputMap::get));
}

public Map<String, Message> mapMessageByControlId(List<HL7FileStream> files)
public Map<String, HapiHL7Message> parseAndMapMessageByControlId(List<HL7FileStream> files)
throws HapiHL7FileMatcherException {

Map<String, Message> messageMap = new HashMap<>();
Map<String, HapiHL7Message> messageMap = new HashMap<>();

try (HapiContext context = new DefaultHapiContext()) {
Parser parser = context.getPipeParser();
Expand All @@ -68,13 +67,13 @@ public Map<String, Message> mapMessageByControlId(List<HL7FileStream> files)
try (InputStream inputStream = hl7FileStream.inputStream()) {
String content = new String(inputStream.readAllBytes(), StandardCharsets.UTF_8);
Message message = parser.parse(content);
MSH mshSegment = (MSH) message.get("MSH");
String msh10 = mshSegment.getMessageControlID().getValue();
HapiHL7Message hapiHL7Message = new HapiHL7Message(message);
String msh10 = hapiHL7Message.getMessageIdentifier();
basiliskus marked this conversation as resolved.
Show resolved Hide resolved
if (msh10 == null || msh10.isEmpty()) {
throw new HapiHL7FileMatcherException(
String.format("MSH-10 is empty for file: %s", fileName));
}
messageMap.put(msh10, message);
messageMap.put(msh10, hapiHL7Message);
} catch (HL7Exception e) {
throw new HapiHL7FileMatcherException(
String.format("Failed to parse HL7 message from file: %s", fileName),
Expand Down
11 changes: 11 additions & 0 deletions rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7Message.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi;

import ca.uhn.hl7v2.HL7Exception;
import ca.uhn.hl7v2.model.Message;
import ca.uhn.hl7v2.model.v251.segment.MSH;
import gov.hhs.cdc.trustedintermediary.wrappers.HealthData;

/**
Expand All @@ -24,4 +26,13 @@ public Message getUnderlyingData() {
public String getName() {
return underlyingData.getName();
}

public String getMessageIdentifier() {
try {
MSH mshSegment = (MSH) underlyingData.get("MSH");
return mshSegment.getMessageControlID().getValue();
} catch (HL7Exception e) {
return null;
}
}
}
12 changes: 4 additions & 8 deletions ...e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
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.wrappers.HealthData;
import gov.hhs.cdc.trustedintermediary.wrappers.Logger;
import gov.hhs.cdc.trustedintermediary.wrappers.formatter.TypeReference;
import java.io.IOException;
Expand Down Expand Up @@ -64,21 +63,18 @@ public List<AssertionRule> getRules() {
return assertionRules;
}

public Set<AssertionRule> runRules(Message outputMessage, Message inputMessage) {
public Set<AssertionRule> runRules(HealthData<?> outputMessage, HealthData<?> inputMessage) {
try {
ensureRulesLoaded();
} catch (RuleLoaderException e) {
logger.logError("Failed to load rules definitions", e);
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);
if (rule.shouldRun(outputMessage)) {
rule.runRule(outputMessage, inputMessage);
runRules.add(rule);
}
}
Expand Down
6 changes: 3 additions & 3 deletions rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ class AutomatedTest extends Specification {

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

Expand Down
18 changes: 9 additions & 9 deletions .../groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ class HapiHL7FileMatcherTest extends Specification {
def mockInputMessage2 = Mock(Message)
def mockOutputMessage1 = Mock(Message)
def mockOutputMessage2 = Mock(Message)
spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "001": mockInputMessage1, "002": mockInputMessage2 ]
spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "001": mockOutputMessage1, "002": mockOutputMessage2 ]
spyFileMatcher.parseAndMapMessageByControlId(mockInputFiles) >> ["001": mockInputMessage1, "002": mockInputMessage2 ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't parseAndMapMessageByControlId now returning a map of string to HapiHL7Message now? This test here, and below, are still returning a map of a string to Message. Shouldn't this be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I'll update it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

spyFileMatcher.parseAndMapMessageByControlId(mockOutputFiles) >> ["001": mockOutputMessage1, "002": mockOutputMessage2 ]

when:
def result =spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)
Expand All @@ -61,8 +61,8 @@ class HapiHL7FileMatcherTest extends Specification {
mockOutputFiles = [
new HL7FileStream("matchingOutputFileStream", Mock(InputStream))
]
spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "001": Mock(Message), "002": Mock(Message) ]
spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "001": Mock(Message) ]
spyFileMatcher.parseAndMapMessageByControlId(mockInputFiles) >> ["001": Mock(Message), "002": Mock(Message) ]
spyFileMatcher.parseAndMapMessageByControlId(mockOutputFiles) >> ["001": Mock(Message) ]
spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)

then:
Expand All @@ -80,8 +80,8 @@ class HapiHL7FileMatcherTest extends Specification {
new HL7FileStream("matchingOutputFileStream", Mock(InputStream)),
new HL7FileStream("nonMatchingOutputFileStream", Mock(InputStream))
]
spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "001": Mock(Message) ]
spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "001": Mock(Message), "003": Mock(Message) ]
spyFileMatcher.parseAndMapMessageByControlId(mockInputFiles) >> ["001": Mock(Message) ]
spyFileMatcher.parseAndMapMessageByControlId(mockOutputFiles) >> ["001": Mock(Message), "003": Mock(Message) ]
spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)

then:
Expand Down Expand Up @@ -110,7 +110,7 @@ class HapiHL7FileMatcherTest extends Specification {
]

when:
def result = fileMatcher.mapMessageByControlId(mockFiles)
def result = fileMatcher.parseAndMapMessageByControlId(mockFiles)

then:
result.size() == 2
Expand All @@ -130,7 +130,7 @@ class HapiHL7FileMatcherTest extends Specification {
def hl7FileStream = new HL7FileStream("file1", inputStream)

when:
fileMatcher.mapMessageByControlId([hl7FileStream])
fileMatcher.parseAndMapMessageByControlId([hl7FileStream])

then:
thrown(HapiHL7FileMatcherException)
Expand All @@ -142,7 +142,7 @@ class HapiHL7FileMatcherTest extends Specification {
def hl7FileStream = new HL7FileStream("badFile", inputStream)

when:
fileMatcher.mapMessageByControlId([hl7FileStream])
fileMatcher.parseAndMapMessageByControlId([hl7FileStream])

then:
thrown(HapiHL7FileMatcherException)
Expand Down
Loading