#57 Accept-ACK (original mode) logic incorrect

[R.]

If MSH-15 is empty on an ACK, then no ACK for that ACK
is expected. The attached patch fixes this logic.

[R.]

If MSH-15 is not present or HL7-NULL then don't expect an ACK

[Nico Vannieuwenhuyze]

Logged In: YES
user_id=930358

Hi,

I'm not convinced that your bugfix is correct.

If hl7 original ack mode is used an application ack should
always be returned when using MLLP.

Can you please verify in the HL7 standard and let me know
if I'm wrong ?

Thanks a lot !

Nico

[R.]

Logged In: YES
user_id=726633

First of all, in original mode there are no accept acks,
only application
acks. In the V2.5 specs, Chapter 2, Sections 2.9.2 and 2.9.3
deal with the
acks. Section 2.9.3.3 has this note (I quote):

Note: The original acknowledgment protocol is equivalent
to the
enhanced acknowledgment protocol with MSH-15-accept
acknowledgment
type = NE and MSH-16-application acknowledgment type = AL,
and with
the application acknowledgment message defined so that it
never
requires an accept acknowledgment (MSH-15-accept
acknowledgment
type = NE).

This makes it clear that there are no accept acks for
original mode;
and original mode is signaled with both MSH-15 and MSH-16
being NULL or not
present.

In enhanced ack mode, signalled by at least one of MSH-15 or
MSH-16 being
present and non-NULL, there's an accept-ack if MSH-15 is
present, not-NULL,
and not NE (at least that's my reading - I didn't see this
spelled out
explicitly, though).

To summarize:
if MSH-15 and MSH-16 empty/NULL -> no accept ack
if MSH-15 not empty/NULL -> accept-ack
iff MSH15 != NE
if MSH-15 empty/NULL, MSH16 not empty/NULL -> no accept-ack

This translates to

if (needAcceptAck != null && !needAcceptAck.equals("") &&
!needAcceptAck.equals(NE))
expect accept ack

Now, if I misread the spec for enhanced mode, and the logic
for an accept
ack really is "if MSH-15 empty/NULL or MSH-15 != NE", then
things would
read as follows (third condition changed):

if MSH-15 and MSH-16 empty/NULL -> no accept ack
if MSH-15 not empty/NULL -> accept-ack
iff MSH15 != NE
if MSH-15 empty/NULL, MSH16 not empty/NULL -> accept-ack

which translates to

if (needAcceptAck != null && !needAcceptAck.equals("") &&
!needAcceptAck.equals(NE) ||
(needAcceptAck == null || needAcceptAck.equals("")) &&
(needAppAck != null && !needAppAck.equals("")))
expect accept ack

This is probably easier to read as

if (isSet(needAcceptAck) && !needAcceptAck.equals(NE) ||
!isSet(needAcceptAck) && isSet(needAppAck))
expect accept ack

private static final boolean isSet(String f) {
return (f != null && !f.equals(""));
}

[Bryan Tripp]

Logged In: YES
user_id=230410
Originator: NO

It looks like we were treating it as original mode if MSH-15 was null, which isn't quite right. It has been changed to check both MSH-15 and MSH-16. With this change, if both are null we assume original mode and wait for an acknowledgement. The code that does this is the same code that waits for the enhanced-mode accept acknowledgement but I don't think this creates any problems. James will test and include in the next release. Please comment if you see a further problem with this. The code now reads:

String[] fieldPaths = {"MSH-10", "MSH-15", "MSH-16"};
String[] fields = PreParser.getFields(theMessage.getMessage(), fieldPaths);
String controlId = fields[0];
String needAcceptAck = fields[1];
String needAppAck = fields[2];

checkValidAckNeededCode(needAcceptAck);

trySend(myContext.getLocallyDrivenTransportLayer(), theMessage);

boolean originalMode = (needAcceptAck == null && needAppAck == null);
if (originalMode || !needAcceptAck.equals(NE)) {

...

Bryan

[R.]

Logged In: YES
user_id=726633
Originator: YES

Sorry, but this logic is wrong in two respects:

1. original mode does _not_ have an accept ack (only an app ack,
as correctly stated by nicovn). This is the core bug. So the
logic needs to be

if (!originalMode && !needAcceptAck.equals(NE))

And to fix the NPE (if needAcceptAck == null and needAppAck != null)
it actually needs to be

if (!originalMode && needAcceptAck != null && !needAcceptAck.equals(NE))

Which is exactly equivalent to

if (needAcceptAck != null && !needAcceptAck.equals(NE))

2. You don't take ER7 NULL values ("") properly into account.
For that you need

boolean originalMode =
(needAcceptAck == null || needAcceptAck.equals("")) &&
(needAppAck == null || needAppAck.equals(""));

or just

if (needAcceptAck != null && !needAcceptAck.equals("") &&
!needAcceptAck.equals(NE))

[Bryan Tripp]

Logged In: YES
user_id=230410
Originator: NO

There is a null check missing, thank you.

The rest of it may warrant another look, although I don't think original mode made the distinction between accept and app ACKs. (I don't have the standard with me, so I'm sorry if I'm remembering this wrong.) The original mode ACK behaves more like an accept ACK than app ACK (i.e. it's part of the same message exchange), which is why it uses the same code.

I'm not sure what to make of comment 2. I would expect an ER7 null to show up here as "\"\"" (unless PreParser does something I'm forgetting about), but this normally indicates that the receiving system should delete -- I don't think it has any meaning in MSH 15 or 16. Although I guess treating it as empty is reasonable.

I'll leave it with you James unless you want to talk about it.

Bryan

[R.]

Logged In: YES
user_id=726633
Originator: YES

Please look at the section I quoted below. It explicitly says that original
mode is equivalent to MSH-15 = NE. I.e. no accept-ack.

[R.]

Logged In: YES
user_id=726633
Originator: YES

I just ran across a couple things in the spec (v2.5):

1. regarding null's: 2.15.9.16, the note explicitly mentions
null's:

Note: If MSH-15-accept acknowledgment type and MSH-16-application acknowledgment type are omitted (or are both null), the original acknowledgment mode rules are used.

2. regarding ack in original mode: section 2.18.5 provides an
example of request and response in original mode, and there
is only an application ack which in turn has no ack. So,
sending back an (app-level) ack and then waiting for an accept-ack
is definitely wrong. It's also wrong to send a request and wait
for an accept-ack. There just is no accept-ack in original mode,
only application-level ack.