Avoid ignoring exceptions in automated tests

rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java

@@ -6,8 +6,8 @@
 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;
-import gov.hhs.cdc.trustedintermediary.wrappers.Logger;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
@@ -16,7 +16,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import javax.inject.Inject;
+import java.util.stream.Collectors;
 
 /**
  * The HapiHL7FileMatcher class is responsible for matching input and output HL7 files based on the
@@ -26,74 +26,66 @@ public class HapiHL7FileMatcher {
 
     private static final HapiHL7FileMatcher INSTANCE = new HapiHL7FileMatcher();
 
-    @Inject Logger logger;
-
     private HapiHL7FileMatcher() {}
 
     public static HapiHL7FileMatcher getInstance() {
         return INSTANCE;
     }
 
     public Map<Message, Message> matchFiles(
-            List<HL7FileStream> outputFiles, List<HL7FileStream> inputFiles) {
+            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);
 
-        Set<String> unmatchedInputKeys = new HashSet<>(inputMap.keySet());
-        unmatchedInputKeys.removeAll(outputMap.keySet());
-
-        Set<String> unmatchedOutputKeys = new HashSet<>(outputMap.keySet());
-        unmatchedOutputKeys.removeAll(inputMap.keySet());
-
+        Set<String> inputKeys = inputMap.keySet();
+        Set<String> outputKeys = outputMap.keySet();
         Set<String> unmatchedKeys = new HashSet<>();
-        unmatchedKeys.addAll(unmatchedInputKeys);
-        unmatchedKeys.addAll(unmatchedOutputKeys);
+        unmatchedKeys.addAll(Sets.difference(inputKeys, outputKeys));
+        unmatchedKeys.addAll(Sets.difference(outputKeys, inputKeys));
 
         if (!unmatchedKeys.isEmpty()) {
-            logger.logError(
-                    "Found no match for the following messages with MSH-10: " + unmatchedKeys);
+            throw new HapiHL7FileMatcherException(
+                    "Found no match for messages with the following MSH-10 values: "
+                            + unmatchedKeys);
         }
 
-        Map<Message, Message> messageMap = new HashMap<>();
-        inputMap.keySet().retainAll(outputMap.keySet());
-        inputMap.forEach(
-                (key, inputMessage) -> {
-                    Message outputMessage = outputMap.get(key);
-                    messageMap.put(inputMessage, outputMessage);
-                });
-
-        return messageMap;
+        return inputKeys.stream().collect(Collectors.toMap(inputMap::get, outputMap::get));
     }
 
-    public Map<String, Message> mapMessageByControlId(List<HL7FileStream> files) {
+    public Map<String, Message> mapMessageByControlId(List<HL7FileStream> files)
+            throws HapiHL7FileMatcherException {
 
         Map<String, Message> messageMap = new HashMap<>();
 
         try (HapiContext context = new DefaultHapiContext()) {
             Parser parser = context.getPipeParser();
 
             for (HL7FileStream hl7FileStream : files) {
+                String fileName = hl7FileStream.fileName();
                 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();
                     if (msh10 == null || msh10.isEmpty()) {
-                        logger.logError("MSH-10 is empty for : " + hl7FileStream.fileName());
-                        continue;
+                        throw new HapiHL7FileMatcherException(
+                                String.format("MSH-10 is empty for file: %s", fileName));
                     }
                     messageMap.put(msh10, message);
-                } catch (IOException | HL7Exception e) {
-                    logger.logError(
-                            "An error occurred while parsing the message: "
-                                    + hl7FileStream.fileName(),
+                } catch (HL7Exception e) {
+                    throw new HapiHL7FileMatcherException(
+                            String.format("Failed to parse HL7 message from file: %s", fileName),
                             e);
+                } catch (IOException e) {
+                    throw new HapiHL7FileMatcherException(
+                            String.format("Failed to read file: %s", fileName), e);
                 }
             }
         } catch (IOException e) {
-            logger.logError("An error occurred while constructing the DefaultHapiContext", e);
+            throw new HapiHL7FileMatcherException("Failed to close input stream", e);
         }
 
         return messageMap;

rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherException.java

@@ -0,0 +1,16 @@
+package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi;
+
+/**
+ * The HapiHL7FileMatcherException class is responsible for handling exceptions that occur in the
+ * HapiHL7FileMatcher class.
+ */
+public class HapiHL7FileMatcherException extends Exception {
+
+    public HapiHL7FileMatcherException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public HapiHL7FileMatcherException(String message) {
+        super(message);
+    }
+}

rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherExceptionTest.groovy

@@ -0,0 +1,30 @@
+package gov.hhs.cdc.trustedintermediary.rse2e.external.hapi
+
+import spock.lang.Specification
+
+class HapiHL7FileMatcherExceptionTest  extends Specification {
+
+    def "two param constructor works"() {
+        given:
+        def message = "something blew up!"
+        def cause = new NullPointerException()
+
+        when:
+        def exception = new HapiHL7FileMatcherException(message, cause)
+
+        then:
+        exception.getMessage() == message
+        exception.getCause() == cause
+    }
+
+    def "single param constructor works"() {
+        given:
+        def message = "something blew up!"
+
+        when:
+        def exception = new HapiHL7FileMatcherException(message)
+
+        then:
+        exception.getMessage() == message
+    }
+}

rs-e2e/src/test/groovy/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcherTest.groovy

@@ -20,26 +20,76 @@ class HapiHL7FileMatcherTest extends Specification {
         TestApplicationContext.injectRegisteredImplementations()
     }
 
-    def "should correctly match input and output files and log unmatched files"() {
+    def "should correctly match input and output files"() {
         given:
         def spyFileMatcher = Spy(HapiHL7FileMatcher.getInstance())
-        def fileStream1 = new HL7FileStream("file1", Mock(InputStream))
-        def fileStream2 = new HL7FileStream("file2", Mock(InputStream))
-        def fileStream3 = new HL7FileStream("file3", Mock(InputStream))
-        def mockInputFiles = [fileStream1, fileStream2]
-        def mockOutputFiles = [fileStream2, fileStream3]
+        def mockInputFiles = [
+            new HL7FileStream("inputFileStream1", Mock(InputStream)),
+            new HL7FileStream("inputFileStream2", Mock(InputStream))
+        ]
+        def mockOutputFiles = [
+            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) >> [ "1": Mock(Message), "2": mockInputMessage2 ]
-        spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "2": mockOutputMessage2, "3": Mock(Message) ]
+        spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "001": mockInputMessage1, "002": mockInputMessage2 ]
+        spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "001": mockOutputMessage1, "002": mockOutputMessage2 ]
+
+        when:
+        def result =spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)
+
+        then:
+        result.size() == 2
+        result == Map.of(mockInputMessage1, mockOutputMessage1, mockInputMessage2, mockOutputMessage2)
+    }
+
+
+    def "should throw HapiHL7FileMatcherException if didn't find a match for at least one file in either input or output"() {
+        given:
+        def mockInputFiles
+        def mockOutputFiles
+        def spyFileMatcher = Spy(HapiHL7FileMatcher.getInstance())
+
+        when:
+        mockInputFiles = [
+            new HL7FileStream("nonMatchingInputFileStream", Mock(InputStream)),
+            new HL7FileStream("matchingInputFileStream", Mock(InputStream))
+        ]
+        mockOutputFiles = [
+            new HL7FileStream("matchingOutputFileStream", Mock(InputStream))
+        ]
+        spyFileMatcher.mapMessageByControlId(mockInputFiles) >> [ "001": Mock(Message), "002": Mock(Message) ]
+        spyFileMatcher.mapMessageByControlId(mockOutputFiles) >> [ "001": Mock(Message) ]
+        spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)
+
+        then:
+        def nonMatchingInputException = thrown(HapiHL7FileMatcherException)
+        with(nonMatchingInputException.getMessage()) {
+            contains("Found no match")
+            contains("002")
+        }
 
         when:
-        def result = spyFileMatcher.matchFiles(mockOutputFiles, mockInputFiles)
+        mockInputFiles = [
+            new HL7FileStream("matchingInputFileStream", Mock(InputStream))
+        ]
+        mockOutputFiles = [
+            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.matchFiles(mockOutputFiles, mockInputFiles)
 
         then:
-        result.size() == 1
-        result == Map.of(mockInputMessage2, mockOutputMessage2)
-        1 * mockLogger.logError({ it.contains("Found no match") && it.contains("1") && it.contains("3") })
+        def nonMatchingOutputException = thrown(HapiHL7FileMatcherException)
+        with(nonMatchingOutputException.getMessage()) {
+            contains("Found no match")
+            contains("003")
+        }
     }
 
     def "should map message by control ID"() {
@@ -70,7 +120,7 @@ class HapiHL7FileMatcherTest extends Specification {
         file2MshSegment == result[file2Msh10].encode().trim()
     }
 
-    def "should log an error and continue when MSH-10 is empty"() {
+    def "should throw HapiHL7FileMatcherException when MSH-10 is empty"() {
         given:
         def msh1to9 = "MSH|^~\\&|Sender Application^sender.test.com^DNS|Sender Facility^0.0.0.0.0.0.0.0^ISO|Receiver Application^0.0.0.0.0.0.0.0^ISO|Receiver Facility^simulated-lab-id^DNS|20230101010000-0000||ORM^O01^ORM_O01|"
         def msh11to12 = "|T|2.5.1"
@@ -80,23 +130,21 @@ class HapiHL7FileMatcherTest extends Specification {
         def hl7FileStream = new HL7FileStream("file1", inputStream)
 
         when:
-        def result = fileMatcher.mapMessageByControlId([hl7FileStream])
+        fileMatcher.mapMessageByControlId([hl7FileStream])
 
         then:
-        result.size() == 0
-        1 * mockLogger.logError({ it.contains("MSH-10 is empty") })
+        thrown(HapiHL7FileMatcherException)
     }
 
-    def "should log an error when not able to parse the file as HL7 message"() {
+    def "should throw HapiHL7FileMatcherException when not able to parse the file as HL7 message"() {
         given:
         def inputStream = new ByteArrayInputStream("".bytes)
         def hl7FileStream = new HL7FileStream("badFile", inputStream)
 
         when:
-        def result = fileMatcher.mapMessageByControlId([hl7FileStream])
+        fileMatcher.mapMessageByControlId([hl7FileStream])
 
         then:
-        result.size() == 0
-        1 * mockLogger.logError({ it.contains("An error occurred while parsing the message") }, _ as Exception)
+        thrown(HapiHL7FileMatcherException)
     }
 }