Platform/angela/14116/read hl7 #16859

adegolier · 2024-12-20T00:29:18Z

This PR ...

Removes HL7Reader.getMessages() in favor of HL7Reader.parseHL7Message()

Test Steps:

Run tests

Changes

Removed getMessages() from HL7Reader and changed references to it to use HL7Reader.parseHL7Message()
Removed getMessageModelClasses() from HL7Reader because it was only used by getMessages
Moved additional functions from HL7Reader into the companion object since there is no need to have multiple instances of HL7Reader
Removed HL7Reader.messageToConfigMap because it was no longer being used
Marked profileDirectoryMap and getMessageProfile() in HL7Reader as deprecated as they are only used in the CLI tool (which was outside the scope of the ticket) and added corresponding suppression of deprecated warnings for their usage to ProcessFhirCommands.convertHl7ToFhir()

Checklist

Testing

Tested locally?
Ran ./prime test or ./gradlew testSmoke against local Docker ReportStream container?
(For Changes to /frontend-react/...) Ran npm run lint:write?
Added tests?

Linked Issues

Fixes As a RS developer, I want a consistent way to read HL7 messages #14116

…erter.getBundles which calls HL7Reader.parseHL7Message but still sorting through the tests

…re about the loss of logging?

…adding a check for the messages being empty and logging an error, and added a test to the FHIRConverterIntegrationTests to ensure that it handles a message that isn't empty but doesn't contain HL7

…() into the companion object in HL7Reader.

…lt parsing to get the message type instead of RS specific parsing

…s deprecated because they are only being used in the CLI and should probably not be used elsewhere. Removing useages from CLI was out of scope for this ticket.

…d added some HL7Reader tests back in with modifications

github-actions · 2024-12-20T00:29:35Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Manifest Files

github-actions · 2024-12-20T00:43:19Z

Test Results

1 265 tests - 4 1 261 ✅ - 4 8m 4s ⏱️ +33s
164 suites ±0 4 💤 ±0
164 files ±0 0 ❌ ±0

Results for commit e44d58d. ± Comparison against base commit e0509fe.

This pull request removes 4 tests.

gov.cdc.prime.router.fhirengine.utils.HL7ReaderTests ‑ get getMessages can parse a message that uses the deprecated CE type in OBX2()
gov.cdc.prime.router.fhirengine.utils.HL7ReaderTests ‑ get getMessages no mapped models()
gov.cdc.prime.router.fhirengine.utils.HL7ReaderTests ‑ get getMessages v27 succeeds()
gov.cdc.prime.router.fhirengine.utils.HL7ReaderTests ‑ test getMessageProfile()

♻️ This comment has been updated with latest results.

github-actions · 2024-12-20T01:10:39Z

Integration Test Results

60 files ±0 60 suites ±0 43m 20s ⏱️ +31s
428 tests ±0 418 ✅ ±0 10 💤 ±0 0 ❌ ±0
431 runs ±0 421 ✅ ±0 10 💤 ±0 0 ❌ ±0

Results for commit e44d58d. ± Comparison against base commit e0509fe.

♻️ This comment has been updated with latest results.

jack-h-wang

Couple quick comments that stood out to me on a quick look, but overall looks good to me!

prime-router/src/main/kotlin/SubmissionReceiver.kt

prime-router/src/main/kotlin/fhirengine/utils/HL7Reader.kt

prime-router/src/main/kotlin/SubmissionReceiver.kt

prime-router/src/main/kotlin/azure/service/SubmissionResponseBuilder.kt

prime-router/src/main/kotlin/fhirengine/utils/HL7Reader.kt

prime-router/src/test/kotlin/fhirengine/azure/FHIRConverterIntegrationTests.kt

prime-router/src/main/kotlin/fhirengine/utils/HL7MessageHelpers.kt

…e only otherwise part of the getMessages() saga

adegolier · 2025-01-07T15:12:58Z

Made changes that were previously logged as other tickets, still waiting for code owner approval.

arnejduranovic · 2025-01-07T15:57:27Z

prime-router/src/main/kotlin/SubmissionReceiver.kt

-                val messages = HL7Reader(actionLogs).getMessages(content)
-                val isBatch = HL7Reader(actionLogs).isBatch(content, messages.size)
-                // create a Report for this incoming HL7 message to use for tracking in the database
+                val messageCount = HL7MessageHelpers.messageCount(content)


The changes in this file look good. However, Long-term, we don't want to be doing this type of validation in the receive step (the submission microservice doesn't, see submissions/src/main/kotlin/gov/cdc/prime/reportstream/submissions/controllers/SubmissionController.kt). I think its fine to leave this as is for now and if we find we are not as close to moving to the microservice as we think we are we can prioritize tickets to align this function with the microservice.

prime-router/src/main/kotlin/cli/ProcessFhirCommands.kt

arnejduranovic · 2025-01-07T16:47:49Z

prime-router/src/main/kotlin/azure/service/SubmissionResponseBuilder.kt

-            if (!isBatch && messages.size == 1) {
-                val message = messages.first()
+            if (!isBatch && messageCount == 1) {
+                val message = HL7Reader.parseHL7Message(requestBody)


I think we need to be careful here. parseHL7Message can return exceptions, so what does that mean for the submission api in general? If there is a failure here, it will except out of the main processRequest function and an HTTP error will be returned in from the caller submitToWaters. The DB state will be updated I think by this point, but the queueMessage for the convert step will not be populated.

I poked around a bit and I think this is fine. This submission method is slated to be deprecated and we should take this issue into consideration when implementing this HL7v2 ACK functionality into the submission microservice.

I updated the DevNotes section of this ticket linking to this comment: #15731

This should be okay because we're checking the message count with the iterator and it would have gotten angry there and returned with 0 messages if it was bad HL7.

sonarqubecloud · 2025-01-08T19:52:02Z

Quality Gate failed

Failed conditions
68.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

adegolier added 15 commits December 6, 2024 18:41

[WIP] Changed HL7Reader.getMessages to use the same logic as FHIRConv…

2dc6a1e

…erter.getBundles which calls HL7Reader.parseHL7Message but still sorting through the tests

Merge branch 'main' into platform/angela/14116/ReadHL7

d91b689

Merge branch 'main' into platform/angela/14116/ReadHL7

de360f6

[WIP] tests still broken, I think this is touching CP stuff, do we ca…

7edf98e

…re about the loss of logging?

[WIP] tests still failing, but fixed the SubmissionReceiver tests by …

78807d0

…adding a check for the messages being empty and logging an error, and added a test to the FHIRConverterIntegrationTests to ensure that it handles a message that isn't empty but doesn't contain HL7

[WIP] Still working on tests. Fixed HL7AckUtilsTest and moved isBatch…

27bc350

…() into the companion object in HL7Reader.

[WIP] Tests still broken, fixed FHIRBundleHelpersTests by using defau…

e074084

…lt parsing to get the message type instead of RS specific parsing

[WIP] Fixed HL7Reader tests and marked a couple things in HL7Reader a…

37b41b7

…s deprecated because they are only being used in the CLI and should probably not be used elsewhere. Removing useages from CLI was out of scope for this ticket.

[WIP] Added error handling to HL7MessageHelpers for batching files

68640e8

Dealing with merge conflicts

67c8df4

More effort to deal with merge conflicts

cc2b64c

Merge branch 'main' into platform/angela/14116/ReadHL7

ad37042

One more little leftover from merge conflicts

0360fba

Fixed some more references to getMessages() that came in from main an…

7bb94b1

…d added some HL7Reader tests back in with modifications

Minor fix to HL7MessageHelpers

955f3f8

adegolier requested a review from a team as a code owner December 20, 2024 00:29

Merge branch 'main' into platform/angela/14116/ReadHL7

97600ba

adegolier temporarily deployed to staging December 20, 2024 00:29 — with GitHub Actions Inactive

adegolier added the platform Platform Team label Dec 20, 2024

jack-h-wang requested changes Dec 20, 2024

View reviewed changes

prime-router/src/main/kotlin/SubmissionReceiver.kt Outdated Show resolved Hide resolved

prime-router/src/main/kotlin/fhirengine/utils/HL7Reader.kt Show resolved Hide resolved

arnejduranovic reviewed Dec 20, 2024

View reviewed changes

added requested helper function

0e5b19d

adegolier temporarily deployed to staging December 23, 2024 16:49 — with GitHub Actions Inactive

removed unnecessary test

fc64067

adegolier temporarily deployed to staging December 23, 2024 16:57 — with GitHub Actions Inactive

jack-h-wang reviewed Dec 23, 2024

View reviewed changes

prime-router/src/main/kotlin/fhirengine/utils/HL7MessageHelpers.kt Show resolved Hide resolved

jack-h-wang approved these changes Dec 26, 2024

View reviewed changes

Removed MessageType validation from SubmissionReceiver

306d985

adegolier temporarily deployed to staging January 6, 2025 22:21 — with GitHub Actions Inactive

adegolier mentioned this pull request Jan 6, 2025

Remove validation from SubmissionReceiver #16902

Closed

1 task

Removed HL7MessageParseAndConvertConfiguration

f36a90a

adegolier temporarily deployed to staging January 6, 2025 23:17 — with GitHub Actions Inactive

adegolier mentioned this pull request Jan 6, 2025

Remove HL7MessageParseAndConvertConfiguration as a concept #16904

Closed

1 task

Modified ProcessFhirCommands to remove things from HL7Reader that wer…

08357af

…e only otherwise part of the getMessages() saga

adegolier temporarily deployed to staging January 7, 2025 00:04 — with GitHub Actions Inactive

adegolier mentioned this pull request Jan 7, 2025

Remove old code from HL7Reader that is used places other than UP #16901

Closed

2 tasks

Merge branch 'main' into platform/angela/14116/ReadHL7

f64fe7e

adegolier temporarily deployed to staging January 7, 2025 15:01 — with GitHub Actions Inactive

arnejduranovic approved these changes Jan 7, 2025

View reviewed changes

arnejduranovic mentioned this pull request Jan 7, 2025

Implement HL7v2 Message Response in Submission API #15731

Open

1 task

adegolier and others added 2 commits January 7, 2025 12:17

removed deprecation mark from ProcessFhirCommands

44c93c8

Merge branch 'main' into platform/angela/14116/ReadHL7

7d6422f

adegolier temporarily deployed to staging January 7, 2025 17:48 — with GitHub Actions Inactive

Merge branch 'main' into platform/angela/14116/ReadHL7

f511958

adegolier enabled auto-merge (squash) January 7, 2025 18:24

adegolier temporarily deployed to staging January 7, 2025 18:31 — with GitHub Actions Inactive

Merge branch 'main' into platform/angela/14116/ReadHL7

e44d58d

adegolier temporarily deployed to staging January 8, 2025 19:30 — with GitHub Actions Inactive

adegolier merged commit aeb20c7 into main Jan 8, 2025
24 of 25 checks passed

adegolier deleted the platform/angela/14116/ReadHL7 branch January 8, 2025 20:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Platform/angela/14116/read hl7 #16859

Platform/angela/14116/read hl7 #16859

adegolier commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

jack-h-wang left a comment

adegolier commented Jan 7, 2025

arnejduranovic Jan 7, 2025

arnejduranovic Jan 7, 2025

arnejduranovic Jan 7, 2025

adegolier Jan 7, 2025 •

edited

Loading

sonarqubecloud bot commented Jan 8, 2025

Platform/angela/14116/read hl7 #16859

Platform/angela/14116/read hl7 #16859

Conversation

adegolier commented Dec 20, 2024 • edited Loading

Changes

Checklist

Testing

Linked Issues

github-actions bot commented Dec 20, 2024 • edited Loading

Dependency Review

OpenSSF Scorecard

Scanned Manifest Files

github-actions bot commented Dec 20, 2024 • edited Loading

Test Results

github-actions bot commented Dec 20, 2024 • edited Loading

Integration Test Results

jack-h-wang left a comment

Choose a reason for hiding this comment

adegolier commented Jan 7, 2025

arnejduranovic Jan 7, 2025

Choose a reason for hiding this comment

arnejduranovic Jan 7, 2025

Choose a reason for hiding this comment

arnejduranovic Jan 7, 2025

Choose a reason for hiding this comment

adegolier Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 8, 2025

Quality Gate failed

adegolier commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

github-actions bot commented Dec 20, 2024 •

edited

Loading

adegolier Jan 7, 2025 •

edited

Loading