Refactor HapiHL7Message.getName to return more useful identifier

[basiliskus]

Description

This PR removes HapiHL7Message.getName method and adds HapiHL7Message.getIdentifier which returns MSH-10 that identifies the message. Using this value we can now identify which test message failed for a specific rule in the Automated Test failure logs

Before:
Assertion failed for rule (ORU)

After:
Assertion failed for rule (009)

There is also some refactoring to better isolate the Hapi HL7 library using the HapiHL7Message wrapper and HealthData interface

Issue

#1621

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1621 - Partially compliant Fully compliant requirements: Refactor HapiHL7Message.getName to return more useful name Not compliant requirements: The identifier name could be the file name, MSH-10 or something else Use a name that can be used to log a message that had an issue

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Bug The refactoring changes the method signature and behavior which might affect existing functionalities that depend on the old method.

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add null check for msh10 before using it as a key to prevent NullPointerException Ensure that the parseAndMapMessageByControlId method handles potential NullPointerException when msh10 is null before using it as a key in messageMap. rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java [72-76] if (msh10 == null || msh10.isEmpty()) { throw new HapiHL7FileMatcherException( String.format("MSH-10 is empty for file: %s", fileName)); +} else { + messageMap.put(msh10, hapiHL7Message); } -messageMap.put(msh10, hapiHL7Message); Suggestion importance[1-10]: 8 Why: This suggestion correctly identifies a potential NullPointerException due to the use of msh10 as a key in a map without ensuring it's not null. Implementing this check enhances the robustness and reliability of the code.	8
	Add null check for mshSegment to prevent NullPointerException Handle the potential NullPointerException that could occur if mshSegment is null after attempting to retrieve it from underlyingData. rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7Message.java [32-33] MSH mshSegment = (MSH) underlyingData.get("MSH"); -return mshSegment.getMessageControlID().getValue(); +if (mshSegment != null) { + return mshSegment.getMessageControlID().getValue(); +} else { + return null; +} Suggestion importance[1-10]: 8 Why: The suggestion is valid as it addresses a potential NullPointerException when mshSegment could be null. This check ensures the stability of the method by preventing runtime exceptions.	8
	Add exception handling for IOException during byte reading to enhance robustness Ensure that the parseAndMapMessageByControlId method properly handles IOException that might be thrown when reading bytes from the InputStream. rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java [68-69] -String content = new String(inputStream.readAllBytes(), StandardCharsets.UTF_8); +String content; +try { + content = new String(inputStream.readAllBytes(), StandardCharsets.UTF_8); +} catch (IOException e) { + throw new HapiHL7FileMatcherException( + String.format("Failed to read bytes from file: %s", fileName), e); +} Message message = parser.parse(content); Suggestion importance[1-10]: 7 Why: This suggestion is relevant as it adds necessary exception handling for IOException which could occur during byte reading from an InputStream. This improvement ensures that the method can gracefully handle I/O errors.	7
	Add duplicate key check to prevent data overwrites in the map Validate that the parseAndMapMessageByControlId method correctly handles cases where msh10 might be duplicated across multiple files, potentially causing data overwrite in messageMap. rs-e2e/src/main/java/gov/hhs/cdc/trustedintermediary/rse2e/external/hapi/HapiHL7FileMatcher.java [76] -messageMap.put(msh10, hapiHL7Message); +if (messageMap.containsKey(msh10)) { + throw new HapiHL7FileMatcherException( + String.format("Duplicate MSH-10 identifier found for file: %s", fileName)); +} else { + messageMap.put(msh10, hapiHL7Message); +} Suggestion importance[1-10]: 7 Why: The suggestion to check for duplicate msh10 values before putting them in the map is crucial to avoid data integrity issues. This check prevents overwriting existing entries, which could lead to data loss or incorrect data processing.	7

[halprin]

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?

[basiliskus]

Good catch. I'll update it

[basiliskus]

Done

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[halprin]

Looking good.