Refactor HapiHL7Message.getName to return more useful identifier (#…

…1636) * WIP: Refactored to avoid leaking external dependency + some cleanup * Renamed HapiHL7Message.getName => getIdentifier * Fixed failing tests * Fixed flipped parameters * Missed updating Message => HapiHL7Message in HapiHL7FileMatcherTest * Cleanup
CDCgov · Dec 3, 2024 · 1a34f32 · 1a34f32
1 parent 421711f
commit 1a34f32
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 57 deletions.
diff --git a/...src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java b/...src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java
@@ -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;
@@ -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();
@@ -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();
@@ -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.getIdentifier();
                     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),

diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7Message.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7Message.java
@@ -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;
 
 /**
@@ -21,7 +23,12 @@ public Message getUnderlyingData() {
     }
 
     @Override
-    public String getName() {
-        return underlyingData.getName();
+    public String getIdentifier() {
+        try {
+            MSH mshSegment = (MSH) underlyingData.get("MSH");
+            return mshSegment.getMessageControlID().getValue();
+        } catch (HL7Exception e) {
+            return null;
+        }
     }
 }
diff --git a/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRule.java b/rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRule.java
@@ -46,7 +46,7 @@ public final void runRule(HealthData<?>... data) {
                                     + "': "
                                     + assertion
                                     + " ("
-                                    + outputData.getName()
+                                    + outputData.getIdentifier()
                                     + ")");
                 }
             } catch (Exception e) {

diff --git a/...e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngine.java b/...e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngine.java
@@ -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;
@@ -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);
             }
         }

diff --git a/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy b/rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/AutomatedTest.groovy
@@ -1,6 +1,5 @@
 package gov.hhs.cdc.trustedintermediary.rse2e
 
-import ca.uhn.hl7v2.model.Message
 import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext
 import gov.hhs.cdc.trustedintermediary.rse2e.external.hapi.HapiHL7FileMatcher
 import gov.hhs.cdc.trustedintermediary.rse2e.external.hapi.HapiHL7ExpressionEvaluator
@@ -65,8 +64,8 @@ class AutomatedTest extends Specification  {
 
         when:
         for (messagePair in matchedFiles) {
-            Message inputMessage = messagePair.getKey() as Message
-            Message outputMessage = messagePair.getValue() as Message
+            def inputMessage = messagePair.getKey()
+            def outputMessage = messagePair.getValue()
             def evaluatedRules = engine.runRules(outputMessage, inputMessage)
             rulesToEvaluate.removeAll(evaluatedRules)
         }

diff --git a/.../groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy b/.../groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy
@@ -1,6 +1,5 @@
 package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi
 
-import ca.uhn.hl7v2.model.Message
 import gov.hhs.cdc.trustedintermediary.context.TestApplicationContext
 import gov.hhs.cdc.trustedintermediary.rse2e.HL7FileStream
 import gov.hhs.cdc.trustedintermediary.wrappers.Logger
@@ -31,15 +30,15 @@ class HapiHL7FileMatcherTest extends Specification {
             new HL7FileStream("outputFileStream1", Mock(InputStream)),
             new HL7FileStream("outputFileStream2", Mock(InputStream))
         ]
-        def mockInputMessage1 = Mock(Message)
-        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 ]
+        def mockInputMessage1 = Mock(HapiHL7Message)
+        def mockInputMessage2 = Mock(HapiHL7Message)
+        def mockOutputMessage1 = Mock(HapiHL7Message)
+        def mockOutputMessage2 = Mock(HapiHL7Message)
+        spyFileMatcher.parseAndMapMessageByControlId(mockInputFiles) >> ["001": mockInputMessage1, "002": mockInputMessage2 ]
+        spyFileMatcher.parseAndMapMessageByControlId(mockOutputFiles) >> ["001": mockOutputMessage1, "002": mockOutputMessage2 ]
 
         when:
-        def result =spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)
+        def result = spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)
 
         then:
         result.size() == 2
@@ -61,8 +60,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(HapiHL7Message), "002": Mock(HapiHL7Message) ]
+        spyFileMatcher.parseAndMapMessageByControlId(mockOutputFiles) >> ["001": Mock(HapiHL7Message) ]
         spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)
 
         then:
@@ -80,8 +79,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(HapiHL7Message) ]
+        spyFileMatcher.parseAndMapMessageByControlId(mockOutputFiles) >> ["001": Mock(HapiHL7Message), "003": Mock(HapiHL7Message) ]
         spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)
 
         then:
@@ -110,14 +109,16 @@ class HapiHL7FileMatcherTest extends Specification {
         ]
 
         when:
-        def result = fileMatcher.mapMessageByControlId(mockFiles)
+        def result = fileMatcher.parseAndMapMessageByControlId(mockFiles)
 
         then:
         result.size() == 2
-        result[file1Msh10] != null
-        file1MshSegment == result[file1Msh10].encode().trim()
-        result[file2Msh10] != null
-        file2MshSegment == result[file2Msh10].encode().trim()
+        def message1 = result[file1Msh10]
+        def message2 = result[file2Msh10]
+        message1 != null
+        message2 != null
+        file1MshSegment == message1.getUnderlyingData().encode().trim()
+        file2MshSegment == message2.getUnderlyingData().encode().trim()
     }
 
     def "should throw HapiHL7FileMatcherException when MSH-10 is empty"() {
@@ -130,7 +131,7 @@ class HapiHL7FileMatcherTest extends Specification {
         def hl7FileStream = new HL7FileStream("file1", inputStream)
 
         when:
-        fileMatcher.mapMessageByControlId([hl7FileStream])
+        fileMatcher.parseAndMapMessageByControlId([hl7FileStream])
 
         then:
         thrown(HapiHL7FileMatcherException)
@@ -142,7 +143,7 @@ class HapiHL7FileMatcherTest extends Specification {
         def hl7FileStream = new HL7FileStream("badFile", inputStream)
 
         when:
-        fileMatcher.mapMessageByControlId([hl7FileStream])
+        fileMatcher.parseAndMapMessageByControlId([hl7FileStream])
 
         then:
         thrown(HapiHL7FileMatcherException)

diff --git a/...test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7MessageTest.groovy b/...test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7MessageTest.groovy
@@ -1,11 +1,13 @@
 package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi
 
 import ca.uhn.hl7v2.model.Message
+import ca.uhn.hl7v2.model.v251.message.ORU_R01
+import ca.uhn.hl7v2.model.v251.segment.MSH
 import spock.lang.Specification
 
 class HapiHL7MessageTest extends Specification {
 
-    def "should correctly initialize and return underlying data"() {
+    def "getUnderlyingData should correctly initialize and return underlying data"() {
         given:
         def mockMessage = Mock(Message)
         def hl7Message = new HapiHL7Message(mockMessage)
@@ -14,19 +16,20 @@ class HapiHL7MessageTest extends Specification {
         hl7Message.getUnderlyingData() == mockMessage
     }
 
-    def "should return the name of the underlying message"() {
+    def "getIdentifier should return the MSH-10 identifier of the underlying message"() {
         given:
-        def expectedName = "TestMessage"
-        def message = Mock(Message)
-        message.getName() >> expectedName
+        def expectedIdentifier = "0001"
+        def oruMessage = new ORU_R01()
+        MSH mshSegment = oruMessage.getMSH()
+        mshSegment.getMessageControlID().setValue(expectedIdentifier)
 
         and:
-        def hl7Message = new HapiHL7Message(message)
+        def hl7Message = new HapiHL7Message(oruMessage)
 
         when:
-        def name = hl7Message.getName()
+        def name = hl7Message.getIdentifier()
 
         then:
-        name == expectedName
+        name == expectedIdentifier
     }
 }
diff --git a/...st/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy b/...st/groovy/gov/hhs/cdc/trustedintermediary/rse2e/ruleengine/AssertionRuleEngineTest.groovy
@@ -85,7 +85,7 @@ class AssertionRuleEngineTest extends Specification {
         }
 
         when:
-        ruleEngine.runRules(Mock(Message), Mock(Message))
+        ruleEngine.runRules(Mock(HapiHL7Message), Mock(HapiHL7Message))
 
         then:
         1 * mockLogger.logError(_ as String, exception)
@@ -97,15 +97,15 @@ class AssertionRuleEngineTest extends Specification {
         mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> { throw exception }
 
         when:
-        ruleEngine.runRules(Mock(Message), Mock(Message))
+        ruleEngine.runRules(Mock(HapiHL7Message), Mock(HapiHL7Message))
 
         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))
+        def result = ruleEngine.runRules(Mock(HapiHL7Message), Mock(HapiHL7Message))
 
         then:
         result.isEmpty()
@@ -117,7 +117,7 @@ class AssertionRuleEngineTest extends Specification {
         mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [rule]
 
         when:
-        def result = ruleEngine.runRules(Mock(Message), Mock(Message))
+        def result = ruleEngine.runRules(Mock(HapiHL7Message), Mock(HapiHL7Message))
 
         then:
         result.size() == 1
@@ -130,7 +130,7 @@ class AssertionRuleEngineTest extends Specification {
         mockRuleLoader.loadRules(_ as InputStream, _ as TypeReference) >> [rule]
 
         when:
-        def result = ruleEngine.runRules(Mock(Message), Mock(Message))
+        def result = ruleEngine.runRules(Mock(HapiHL7Message), Mock(HapiHL7Message))
 
         then:
         result.isEmpty()

diff --git a/shared/src/main/java/gov/hhs/cdc/trustedintermediary/wrappers/HealthData.java b/shared/src/main/java/gov/hhs/cdc/trustedintermediary/wrappers/HealthData.java
@@ -6,7 +6,7 @@
 public interface HealthData<T> {
     T getUnderlyingData();
 
-    default String getName() {
+    default String getIdentifier() {
         return "";
     }
 }
diff --git a/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/wrappers/HealthDataTest.groovy b/shared/src/test/groovy/gov/hhs/cdc/trustedintermediary/wrappers/HealthDataTest.groovy
@@ -8,7 +8,7 @@ class HealthDataTest extends Specification {
         def healthData = new IntegerHealthData(1)
 
         when:
-        def actual = healthData.getName()
+        def actual = healthData.getIdentifier()
 
         then:
         actual == ""