nHapiNET / nHapi

nHapi is the .Net port of the original Java project HAPI.
Mozilla Public License 2.0
277 stars 157 forks source link

ADT^A31 v2.5 message gets parsed as ADT^A05 v2.5 #597

Closed eloekset closed 1 month ago

eloekset commented 1 month ago

Describe the bug new PipeParser().Parse(hl7Content) parses an ADT^A31 message as ADT^A05.

To Reproduce Parse the sample message content.

Expected behaviour This should be recognized as an ADT^A31 v2.5 message.

Screenshots image

Sample HL7

MSH|^~\&|EPIC|HNT||11^983974791^|20200303141713|100545AA|ADT^A31|452572|T|2.5|||NE|NE
EVN|A31|20210202132954||REG_MANUAL|PULTECH^PULMONARY^TECHNOLOGIST^^^^^^STO
PID|1||26028430972^^^NIN^NIN~E6980^^^EPIC^MRN||Awtcv^Fourteenseven||19870803|M|||VAR FRUE GATE 2703^^TRONDHEIM^NO-50^7012^NORGE^P^^5001|5001|96 52 55 30^P^H^^^96^525530~NET^Internet^AWTTEST1@test.com|||UKJ||910011747|26028430972|||||||||||N
PD1|||HNT NAM SYKEHUSET^^HNTNAMSH
PV1|1|O|NAHJERTEP^^^HNTNAMSH^^^^^HNT NAM SH KARDIOLOGI POLIKLINIKK^^IDEPID|||||2118386^SAMSTAD^STEIN^OLAV^^^^^PROVHPR^^^^PROVHPR||150|||||||||487559|EGEN|||||||||||||||||||||HOV*Conf|||20240812145850|||4000|||487559
PV2||||||||20240812145851||||102||||||||||N

Environmental Details (please complete the following information):

Additional context Works fine for ADT^A01 sample message and several other similar sample messages:

MSH|^~\&|EPIC|HNT||11^983974791^|20200303141713|100545AA|ADT^A01|452572|T|2.5|||NE|NE
EVN|A01|20200303141713||REG_PAT_NEW|100545AA^DINOVI^BRETT^ELDEN^^^^^HMR
PID|1||123313131455^^^DUF^DUF~18048201209^^^NIN^NIN||GENERATOR^ADT^^^^^D||19870803|K||White|321 INFORMATION LANE^^^^^^P^^||||NO|SINGLE||||||EURO (FINSK)||N||||||N
PD1|||TEKNOSTALLEN^^TEKNOST|123456^^^^^^^^^^^^PROVHER~123457^^^^^^^^^^^^PROVHPR
PV1|1|N|
eloekset commented 1 month ago

I've downloaded the sources to investigate this, and it's the NameValueCollection.Get() method that returns ADT_A05 even though ADT_A31 is in that collection of keys: image

eloekset commented 1 month ago

I've created a fork to investigate this issue: https://github.com/eloekset/nHapi/tree/feature/issue-597-investigate-parsing

I had to upgrade .NET and various packages to be able to run tests in VS2022, so I did that in a separate commit before adding the unit test used to debug as shown in the screenshot above.

eloekset commented 1 month ago

For some reason the NHapi.Model.V25 assembly has a mapping from ADT_A31 to ADT_A05. image

milkshakeuk commented 1 month ago

@eloekset hello, thanks for raising an issue, have you read the wiki page about event mapping? https://github.com/nHapiNET/nHapi/wiki/Decoding-a-message#event-map-to-code-structure-mappings I think what you might be describing is "a feature" or querk 🙂 rather than a bug.

The reason why is because this is how the Java hapi library works of which this is a port, they have a Faq with a question covering this here.

the event mapping for 2.5 maps A31 to A05 accoring to the map https://github.com/nHapiNET/nHapi/blob/a9f42181815e035383120540160d470c03f60918/src/NHapi.Model.V25/EventMapping/EventMap.properties#L32

Please let me know if I have missunderstood.

milkshakeuk commented 1 month ago

also you fork seems to be 135 commits behind the current nhapi master branch

image

eloekset commented 1 month ago

Thanks! I've found this part of the documentation. I've been trying to make the PipeParser parse ADT^A31 messages as my custom message defined in my application's Core assembly, however I've still not succeeded despite several attempts at debugging the PackageManager and EventMapper code.

There's a comment in the DefaultModelClassFactory describing how to define custom messages in the original Java project, but that doesn't match the nHapi port: https://github.com/nHapiNET/nHapi/blob/a9f42181815e035383120540160d470c03f60918/src/NHapi.Base/Parser/DefaultModelClassFactory.cs#L42

image

I'm trying to define a v2.5 message with a few custom segments: image

milkshakeuk commented 1 month ago

there is a section in the wiki which explains how this can be done https://github.com/nHapiNET/nHapi/wiki/Configuration-Options#custom-hl7-object-models

There is also a unit test showing this in action: https://github.com/nHapiNET/nHapi/blob/master/tests/NHapi.NUnit/CustomZSegmentTest.cs corresponding app.config https://github.com/nHapiNET/nHapi/blob/master/tests/NHapi.NUnit/App.config

eloekset commented 1 month ago

Thanks! Those are the items I've found, and I'm trying to figure out how to override the default mapping from ADT_A31 to ADT_A05 for v2.5. Since I've created a file called EventMap.properties in a folder EventMapping and set its Build Action to Embedded resource, I would expect the dynamic resolving code in the EventMapper class to load the message structure that I've specified for v2.5: image

However that doesn't happen, because the inResource variable resolved to null: image

Since the CustomZSegmentTest doesn't originally define custom messages like I do, this feature isn't demonstrated.

I can try to rebase my feature branch and push the latest changes as far as I've come now.

eloekset commented 1 month ago

I created a new branch instead of rebasing, since the NUnit stuff was already updated for VS2022. I've pushed my latest attempt here: https://github.com/eloekset/nHapi/tree/feature/issue-597-investigate-parsing-2

However, I now got the idea that maybe it's easier to create a custom model factory that derives from DefaultModelClassFactory instead of relying on that dynamic loading of message structures. (It doesn't even get into the constructor of the ADT_A31 message, so the custom model factory won't ever be used.)

milkshakeuk commented 1 month ago

the CustomZSegmentTest does use a custom message https://github.com/nHapiNET/nHapi/blob/master/src/NHapi.Model.V22_ZSegments/Message/ADT_A08.cs you shouldnt need to attempt to override the event mapping.

milkshakeuk commented 1 month ago

did you see this bit in the wiki when parsing?

var parser = new PipeParser();

var parsed = parser.Parse(message, "2.2.CustomZ");

Notice the second argument, it tells the parser to use the custom version when parsing.

eloekset commented 1 month ago

Thanks for the help @milkshakeuk!

This finally did the trick: image