Skip to content

Improve HL7v2 loose parser test coverage #399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NicoPiel wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Improve HL7v2 loose parser test coverage #399

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9a93c48
Add SonarQube
NicoPiel Jul 30, 2026
12298ea
test: run the orphaned HL7v2 loose serializer suite
NicoPiel Jul 30, 2026
8ef628f
test: refresh HL7v2 fixtures for handleSubcomponents default
NicoPiel Jul 30, 2026
e9e7443
test: cover HL7v2 loose parser encoding-character handling
NicoPiel Jul 30, 2026
efcc525
test: cover HL7v2 loose parser degenerate separators
NicoPiel Jul 30, 2026
13a0641
test: cover ER7Serializer.transformWithoutSerializing
NicoPiel Jul 30, 2026
be3e5a2
test: cover ER7Serializer's delimiter-replace branch
NicoPiel Jul 30, 2026
70a3bbe
test: assert HL7v2 loose parser ER7 round-trip invariant
NicoPiel Jul 30, 2026
da8c39b
Clean up ER7SerializerTest and HL7SerializerTest per review
NicoPiel Jul 30, 2026
e353553
Remove SonarQube plugin from build.gradle
NicoPiel Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions server/src/test/java/com/mirth/connect/model/converters/TestUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,12 @@ public static String prettyPrintXml(String input) throws Exception {
public static String convertCRToCRLF(String input) {
return input.replaceAll("\r", "\r\n");
}

/**
* Collapses CRLF/CR/LF to LF and trims surrounding whitespace, so that expected values stored
* in CRLF fixture files compare equal to output whose line separator is platform-dependent.
*/
public static String normalizeLineEndings(String input) {
return input.replaceAll("\r\n|\r|\n", "\n").trim();

@jonbartels jonbartels Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking comment - Is there an Apache Commons method for this?

@NicoPiel NicoPiel Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Checked lang3, commons-io and commons-text.

}
}
152 changes: 151 additions & 1 deletion server/src/test/java/com/mirth/connect/plugins/datatypes/hl7v2/ER7SerializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,95 @@
package com.mirth.connect.plugins.datatypes.hl7v2;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.File;

import org.apache.commons.io.FileUtils;
import org.junit.BeforeClass;
import org.junit.Test;
import org.xml.sax.SAXException;
import org.xml.sax.SAXParseException;

import com.mirth.connect.donkey.model.message.MessageSerializerException;
import com.mirth.connect.model.datatype.SerializerProperties;

public class ER7SerializerTest {
private static final String XML_DECL = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>";
private static final String MSH_ER7 = "MSH|^~\\&|A\r";
private static final String MSH_XML = "<MSH><MSH.1>|</MSH.1><MSH.2>^~\\&amp;</MSH.2><MSH.3><MSH.3.1>A</MSH.3.1></MSH.3></MSH>";

private static ER7Serializer serializer;

@BeforeClass
public static void setupClass() throws Exception {
SerializerProperties serializerProperties = new SerializerProperties(new HL7v2SerializationProperties(), new HL7v2DeserializationProperties(), null);
serializer = new ER7Serializer(serializerProperties);
}

private static ER7Serializer serializerWith(boolean convertLineBreaks, String deserializationSegmentDelimiter) {
HL7v2SerializationProperties serializationProperties = new HL7v2SerializationProperties();
serializationProperties.setConvertLineBreaks(convertLineBreaks);

HL7v2DeserializationProperties deserializationProperties = new HL7v2DeserializationProperties();
deserializationProperties.setSegmentDelimiter(deserializationSegmentDelimiter);

return new ER7Serializer(new SerializerProperties(serializationProperties, deserializationProperties, null));
}

@Test
public void testTransformWithoutSerializingConvertsToOutboundDelimiter() throws Exception {
ER7Serializer inbound = serializerWith(true, "\\r");
ER7Serializer outbound = serializerWith(true, "\\n");

assertEquals("MSH|^~\\&|A\nPID|1", inbound.transformWithoutSerializing("MSH|^~\\&|A\rPID|1", outbound));
}

@Test
public void testTransformWithoutSerializingNormalizesMixedLineBreaks() throws Exception {
ER7Serializer sameEnds = serializerWith(true, "\\r");

assertEquals("MSH|^~\\&|A\rPID|1", sameEnds.transformWithoutSerializing("MSH|^~\\&|A\r\nPID|1", sameEnds));
}

@Test
public void testTransformWithoutSerializingStillConvertsWhenDelimitersMatch() throws Exception {
/*
* convertLineBreaks is on, so the message goes through conversion and is returned even
* though it is unchanged. Only the convertLineBreaks-off case short-circuits to null.
*/
ER7Serializer sameEnds = serializerWith(true, "\\r");

assertEquals("MSH|^~\\&|A\rPID|1", sameEnds.transformWithoutSerializing("MSH|^~\\&|A\rPID|1", sameEnds));
}

@Test
public void testTransformWithoutSerializingReplacesDelimiterWhenLineBreakConversionIsOff() throws Exception {
/*
* The only test that reaches the StringUtils.replace branch at ER7Serializer.java:170.
* Every convertLineBreaks=true configuration short-circuits earlier: either the
* skipIntermediateDelimiter fast path at line 162, or (for the delimiters-match case)
* the convertLineBreaks(...) call at line 165 with transformed already true. Only
* convertLineBreaks=false skips that whole block and falls through to the delimiter
* comparison at line 169.
*/
ER7Serializer inbound = serializerWith(false, "\\r");
ER7Serializer outbound = serializerWith(true, "\\n");

assertEquals("MSH|^~\\&|A\nPID|1", inbound.transformWithoutSerializing("MSH|^~\\&|A\rPID|1", outbound));
}

@Test
public void testTransformWithoutSerializingReturnsNullWhenNothingToDo() throws Exception {
ER7Serializer inbound = serializerWith(false, "\\r");
ER7Serializer outbound = serializerWith(true, "\\r");

assertNull(inbound.transformWithoutSerializing("MSH|^~\\&|A\rPID|1", outbound));
}

@Test
public void testFromXMLWithExternalDTD() throws Exception {
String xml = FileUtils.readFileToString(new File("tests/test-xxe-hl7-example.xml"), "UTF-8");
Expand Down Expand Up @@ -53,4 +121,86 @@ public void testValidFromXMLWithExternalDTD() throws Exception {

assertFalse(exceptionThrown);
}

@Test
public void testToXMLWithCustomEncodingCharacters() throws Exception {
String er7 = "MSH#@%\\$#App1#Fac1\rPID#1#Smith@John";

assertEquals(XML_DECL + "<HL7Message><MSH><MSH.1>#</MSH.1><MSH.2>@%\\$</MSH.2>"
+ "<MSH.3><MSH.3.1>App1</MSH.3.1></MSH.3>"
+ "<MSH.4><MSH.4.1>Fac1</MSH.4.1></MSH.4></MSH>"
+ "<PID><PID.1><PID.1.1>1</PID.1.1></PID.1>"
+ "<PID.2><PID.2.1>Smith</PID.2.1><PID.2.2>John</PID.2.2></PID.2></PID></HL7Message>",
serializer.toXML(er7));
}

@Test
public void testToXMLWithHeaderOnlyAndNoTrailingFieldSeparator() throws Exception {
// Covers ER7Reader's nextDelimiter == -1 branch: MSH-2 runs to end of message.
assertEquals(XML_DECL + "<HL7Message><MSH><MSH.1>|</MSH.1><MSH.2>^~\\&amp;</MSH.2></MSH></HL7Message>",
serializer.toXML("MSH|^~\\&"));
}

@Test
public void testToXMLAppliesMirth1544FixupToNonHeaderFirstSegment() throws Exception {
/*
* Characterization of ER7Reader's MIRTH-1544 fixup. The "^~&|" check is a positional
* substring test that does not require a header segment, so it also fires on a Z-segment,
* installing '&' as the subcomponent separator. Recorded as current behavior, not endorsed.
*/
assertEquals(XML_DECL + "<HL7Message><ZZZ>"
+ "<ZZZ.1><ZZZ.1.1></ZZZ.1.1><ZZZ.1.2></ZZZ.1.2></ZZZ.1>"
+ "<ZZZ.1><ZZZ.1.1><ZZZ.1.1.1></ZZZ.1.1.1><ZZZ.1.1.2></ZZZ.1.1.2></ZZZ.1.1></ZZZ.1>"
+ "<ZZZ.2><ZZZ.2.1><ZZZ.2.1.1>a</ZZZ.2.1.1><ZZZ.2.1.2>b</ZZZ.2.1.2></ZZZ.2.1></ZZZ.2>"
+ "</ZZZ></HL7Message>",
serializer.toXML("ZZZ|^~&|a&b"));

@tonygermano tonygermano Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): update comment to describe expected behavior

I thought there was a bug in your test until I found the "MIRTH-1544" code that tests for the string your comment already calls out (^~&|) and then treats it as if the escape character is \ and missing.

}

@Test
public void testToXMLRejectsMessageShorterThanSixCharacters() throws Exception {
try {
serializer.toXML("MSH");
fail("expected MessageSerializerException");
} catch (MessageSerializerException e) {
assertTrue(e.getCause() instanceof SAXException);
assertEquals("Unable to parse message. It is NULL or too short. MSH", e.getCause().getMessage());
}
}

@Test
public void testToXMLWithConsecutiveRepetitionSeparators() throws Exception {
assertEquals(XML_DECL + "<HL7Message>" + MSH_XML + "<PID>"
+ "<PID.1><PID.1.1>a</PID.1.1></PID.1>"
+ "<PID.1></PID.1>"
+ "<PID.1><PID.1.1>b</PID.1.1></PID.1>"
+ "</PID></HL7Message>",
serializer.toXML(MSH_ER7 + "PID|a~~b"));
}

@Test
public void testToXMLWithTrailingRepetitionSeparator() throws Exception {
assertEquals(XML_DECL + "<HL7Message>" + MSH_XML + "<PID>"
+ "<PID.1><PID.1.1>a</PID.1.1></PID.1>"
+ "<PID.1></PID.1>"
+ "</PID></HL7Message>",
serializer.toXML(MSH_ER7 + "PID|a~"));
}

@Test
public void testToXMLWithTrailingEmptyFields() throws Exception {
assertEquals(XML_DECL + "<HL7Message>" + MSH_XML + "<PID>"
+ "<PID.1><PID.1.1>a</PID.1.1></PID.1>"
+ "<PID.2></PID.2>"
+ "<PID.3></PID.3>"
+ "</PID></HL7Message>",
serializer.toXML(MSH_ER7 + "PID|a||"));
}

@Test
public void testToXMLSkipsEmptySegments() throws Exception {
// StringUtils.split drops empty tokens, so a blank line between segments simply vanishes.
assertEquals(XML_DECL + "<HL7Message>" + MSH_XML
+ "<PID><PID.1><PID.1.1>1</PID.1.1></PID.1></PID></HL7Message>",
serializer.toXML("MSH|^~\\&|A\r\rPID|1"));
}
}
66 changes: 22 additions & 44 deletions ...s/datatypes/hl7v2/HL7SerializerTests.java → ...ns/datatypes/hl7v2/HL7SerializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,27 @@

import java.io.File;

import junit.framework.Assert;

import org.apache.commons.io.FileUtils;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

import com.mirth.connect.model.converters.TestUtil;

public class HL7SerializerTests {
public class HL7SerializerTest {
private HL7v2DataTypeProperties defaultProperties;

@Before
public void setUp() throws Exception {
defaultProperties = new HL7v2DataTypeProperties();

// defaultProperties = new Properties();
// defaultProperties.put("useStrictParser", "false");
// defaultProperties.put("handleRepetitions", "false");
// defaultProperties.put("handleSubcomponents", "false");
// defaultProperties.put("inputSegmentDelimiter", "\\r\\n|\\r|\\n");
// defaultProperties.put("outputSegmentDelimiter", "\\r");
}

@Test
public void testToXmlDefault() throws Exception {
String input = FileUtils.readFileToString(new File("tests/test-hl7-input.txt"));
String output = FileUtils.readFileToString(new File("tests/test-hl7-output.xml"));
ER7Serializer serializer = new ER7Serializer(defaultProperties.getSerializerProperties());
Assert.assertEquals(output, TestUtil.prettyPrintXml(serializer.toXML(input)));
Assert.assertEquals(TestUtil.normalizeLineEndings(output), TestUtil.normalizeLineEndings(TestUtil.prettyPrintXml(serializer.toXML(input))));
}

@Test
Expand All @@ -55,7 +47,7 @@ public void testToXmlWhitepsace() throws Exception {
String input = FileUtils.readFileToString(new File("tests/test-hl7-whitespace-input.txt"));
String output = FileUtils.readFileToString(new File("tests/test-hl7-whitespace-output.xml"));
ER7Serializer serializer = new ER7Serializer(defaultProperties.getSerializerProperties());
Assert.assertEquals(output, TestUtil.prettyPrintXml(serializer.toXML(input)));
Assert.assertEquals(TestUtil.normalizeLineEndings(output), TestUtil.normalizeLineEndings(TestUtil.prettyPrintXml(serializer.toXML(input))));
}

@Test
Expand Down Expand Up @@ -98,11 +90,6 @@ public void testFromXmlMissingSubcomponents() throws Exception {
@Test
public void testFromXmlSingleSegment() throws Exception {
HL7v2DataTypeProperties properties = new HL7v2DataTypeProperties();
// properties.put("useStrictParser", "false");
// properties.put("handleRepetitions", "false");
// properties.put("handleSubcomponents", "true");
// properties.put("inputSegmentDelimiter", "\\r\\n|\\r|\\n");
// properties.put("outputSegmentDelimiter", "\\r");

String input = FileUtils.readFileToString(new File("tests/test-hl7-single-segment-input.xml"));
String output = FileUtils.readFileToString(new File("tests/test-hl7-single-segment-output.txt"));
Expand All @@ -121,28 +108,16 @@ public void testFromXmlSingleField() throws Exception {
@Test
public void testToXmlWithSubcomponents() throws Exception {
HL7v2DataTypeProperties properties = new HL7v2DataTypeProperties();
// Properties properties = new Properties();
// properties.put("useStrictParser", "false");
// properties.put("handleRepetitions", "false");
// properties.put("handleSubcomponents", "true");
// properties.put("inputSegmentDelimiter", "\\r\\n|\\r|\\n");
// properties.put("outputSegmentDelimiter", "\\r");

String input = FileUtils.readFileToString(new File("tests/test-hl7-subcomponents-input.txt"));
String output = FileUtils.readFileToString(new File("tests/test-hl7-subcomponents-output.xml"));
ER7Serializer serializer = new ER7Serializer(properties.getSerializerProperties());
Assert.assertEquals(output, TestUtil.prettyPrintXml(serializer.toXML(input)));
Assert.assertEquals(TestUtil.normalizeLineEndings(output), TestUtil.normalizeLineEndings(TestUtil.prettyPrintXml(serializer.toXML(input))));
}

@Test
public void testFromXmlWithSubcomponents() throws Exception {
HL7v2DataTypeProperties properties = new HL7v2DataTypeProperties();
// Properties properties = new Properties();
// properties.put("useStrictParser", "false");
// properties.put("handleRepetitions", "false");
// properties.put("handleSubcomponents", "true");
// properties.put("inputSegmentDelimiter", "\\r\\n|\\r|\\n");
// properties.put("outputSegmentDelimiter", "\\r");

String input = FileUtils.readFileToString(new File("tests/test-hl7-subcomponents-output.xml"));
String output = FileUtils.readFileToString(new File("tests/test-hl7-subcomponents-input.txt"));
Expand All @@ -153,28 +128,16 @@ public void testFromXmlWithSubcomponents() throws Exception {
@Test
public void testToXmlWithRepetitions() throws Exception {
HL7v2DataTypeProperties properties = new HL7v2DataTypeProperties();
// Properties properties = new Properties();
// properties.put("useStrictParser", "false");
// properties.put("handleRepetitions", "true");
// properties.put("handleSubcomponents", "false");
// properties.put("inputSegmentDelimiter", "\\r\\n|\\r|\\n");
// properties.put("outputSegmentDelimiter", "\\r");

String input = FileUtils.readFileToString(new File("tests/test-hl7-repetitions-input.txt"));
String output = FileUtils.readFileToString(new File("tests/test-hl7-repetitions-output.xml"));
ER7Serializer serializer = new ER7Serializer(properties.getSerializerProperties());
Assert.assertEquals(output, TestUtil.prettyPrintXml(serializer.toXML(input)));
Assert.assertEquals(TestUtil.normalizeLineEndings(output), TestUtil.normalizeLineEndings(TestUtil.prettyPrintXml(serializer.toXML(input))));
}

@Test
public void testFromXmlWithRepetitions() throws Exception {
HL7v2DataTypeProperties properties = new HL7v2DataTypeProperties();
// Properties properties = new Properties();
// properties.put("useStrictParser", "false");
// properties.put("handleRepetitions", "true");
// properties.put("handleSubcomponents", "false");
// properties.put("inputSegmentDelimiter", "\\r\\n|\\r|\\n");
// properties.put("outputSegmentDelimiter", "\\r");

String input = FileUtils.readFileToString(new File("tests/test-hl7-repetitions-output.xml"));
String output = FileUtils.readFileToString(new File("tests/test-hl7-repetitions-input.txt"));
Expand All @@ -187,7 +150,7 @@ public void testToXmlWithBatch() throws Exception {
String input = FileUtils.readFileToString(new File("tests/test-hl7-batch-input.txt"));
String output = FileUtils.readFileToString(new File("tests/test-hl7-batch-output.xml"));
ER7Serializer serializer = new ER7Serializer(defaultProperties.getSerializerProperties());
Assert.assertEquals(output, TestUtil.prettyPrintXml(serializer.toXML(input)));
Assert.assertEquals(TestUtil.normalizeLineEndings(output), TestUtil.normalizeLineEndings(TestUtil.prettyPrintXml(serializer.toXML(input))));
}

@Test
Expand All @@ -197,4 +160,19 @@ public void testFromXmlWithBatch() throws Exception {
ER7Serializer serializer = new ER7Serializer(defaultProperties.getSerializerProperties());
Assert.assertEquals(output, TestUtil.convertCRToCRLF(serializer.fromXML(input)));
}

@Test
public void testRoundTripAllEr7Fixtures() throws Exception {
String[] fixtures = { "tests/test-hl7-input.txt", "tests/test-hl7-whitespace-input.txt",
"tests/test-hl7-subcomponents-input.txt", "tests/test-hl7-repetitions-input.txt",
"tests/test-hl7-batch-input.txt" };

ER7Serializer serializer = new ER7Serializer(defaultProperties.getSerializerProperties());

for (String fixture : fixtures) {
String er7 = FileUtils.readFileToString(new File(fixture));

Assert.assertEquals(fixture, TestUtil.normalizeLineEndings(er7), TestUtil.normalizeLineEndings(serializer.fromXML(serializer.toXML(er7))));
}
}
}
5 changes: 4 additions & 1 deletion server/tests/test-hl7-batch-output.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,10 @@
<OBR.4>
<OBR.4.1>MRS</OBR.4.1>
<OBR.4.2>Shephard</OBR.4.2>
<OBR.4.3>Jane MRI W/&amp;W/O CONTRAST</OBR.4.3>
<OBR.4.3>
<OBR.4.3.1>Jane MRI W/</OBR.4.3.1>
<OBR.4.3.2>W/O CONTRAST</OBR.4.3.2>
Comment on lines +240 to +241

@tonygermano tonygermano Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment (non-blocking): ouch

Apparently the default a long time ago used to be that "parse sub-components" was off. Since the opposite is true now, I understand why this was necessary to fix the test.

But it pains me to look at because, aside from this data strangely including a name mixed with procedure data in OBR-4, I deal with the pain on a regular basis of radiology procedures containing & in the description and not being escaped in the hl7 messages.

</OBR.4.3>
<OBR.4.4>70553</OBR.4.4>
</OBR.4>
<OBR.5/>
Expand Down
5 changes: 4 additions & 1 deletion server/tests/test-hl7-output.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,10 @@
<OBR.4>
<OBR.4.1>MRS</OBR.4.1>
<OBR.4.2>Shephard</OBR.4.2>
<OBR.4.3>Jane MRI W/&amp;W/O CONTRAST</OBR.4.3>
<OBR.4.3>
<OBR.4.3.1>Jane MRI W/</OBR.4.3.1>
<OBR.4.3.2>W/O CONTRAST</OBR.4.3.2>
</OBR.4.3>
<OBR.4.4>70553</OBR.4.4>
</OBR.4>
<OBR.5/>
Expand Down
Loading