Repository: xlate/staedi (java)

Issue:
Validation of edifact fails when an empty segment starts a loop in an implementation schema
If the empty segment is removed (from schemas and edifact file), the validation works perfectly. Same if the empty segment is moved to second or following positions in the loop.

A failed test is attached.

[empty-segment-loop-validation.zip](https://github.com/xlate/staedi/files/5033161/empty-segment-loop-validation.zip)


Pull request:
Correct implementation segment selection at loop start for single match
Fixes #79 

- test case contributed by @benisantos

Patch:
diff --git a/src/main/java/io/xlate/edi/internal/stream/validation/Validator.java b/src/main/java/io/xlate/edi/internal/stream/validation/Validator.java
index 0aa345b..a6407d6 100644
--- a/src/main/java/io/xlate/edi/internal/stream/validation/Validator.java
+++ b/src/main/java/io/xlate/edi/internal/stream/validation/Validator.java
@@ -367,7 +367,7 @@ public class Validator {
         useErrors.clear();
         boolean handled = false;
 
-        while (!handled && cursor.standard != null) {
+        while (!handled) {
             handled = handleNode(tag, cursor.standard, cursor.impl, startDepth, handler);
 
             if (!handled) {
@@ -531,14 +531,7 @@ public class Validator {
         }
 
         completeLoops(handler, startDepth);
-        loopStack.push(current);
-        handler.loopBegin(current.getLink());
-        correctSegment = segment = startLoop(current);
-
-        if (current.exceedsMaximumUsage(SEGMENT_VERSION)) {
-            handleMissingMandatory(handler);
-            handler.segmentError(tag, current.getLink(), LOOP_OCCURS_OVER_MAXIMUM_TIMES);
-        }
+        boolean implUnusedSegment = false;
 
         if (currentImpl != null) {
             UsageNode impl = currentImpl;
@@ -549,13 +542,34 @@ public class Validator {
             }
 
             if (implSegmentCandidates.isEmpty()) {
-                handleMissingMandatory(handler);
-                handler.segmentError(segment.getId(), segment.getLink(), IMPLEMENTATION_UNUSED_SEGMENT_PRESENT);
+                implUnusedSegment = true;
                 // Save the currentImpl so that the search is resumed from the correct location
                 implNode = currentImpl;
+            } else if (implSegmentCandidates.size() == 1) {
+                handleImplementationSelected(currentImpl, currentImpl.getFirstChild(), handler);
             }
         }
 
+        if (currentImpl != null && implSegmentSelected) {
+            loopStack.push(currentImpl);
+            handler.loopBegin(currentImpl.getLink());
+        } else {
+            loopStack.push(current);
+            handler.loopBegin(current.getLink());
+        }
+
+        correctSegment = segment = startLoop(current);
+
+        if (current.exceedsMaximumUsage(SEGMENT_VERSION)) {
+            handleMissingMandatory(handler);
+            handler.segmentError(tag, current.getLink(), LOOP_OCCURS_OVER_MAXIMUM_TIMES);
+        }
+
+        if (implUnusedSegment) {
+            handleMissingMandatory(handler);
+            handler.segmentError(segment.getId(), segment.getLink(), IMPLEMENTATION_UNUSED_SEGMENT_PRESENT);
+        }
+
         return true;
     }
 
diff --git a/src/main/java/io/xlate/edi/stream/EDIValidationException.java b/src/main/java/io/xlate/edi/stream/EDIValidationException.java
index ee03f10..557e46f 100644
--- a/src/main/java/io/xlate/edi/stream/EDIValidationException.java
+++ b/src/main/java/io/xlate/edi/stream/EDIValidationException.java
@@ -59,21 +59,12 @@ public class EDIValidationException extends RuntimeException {
      * @see #getNextException
      */
     public void setNextException(EDIValidationException ex) {
-
         EDIValidationException current = this;
-        for(;;) {
-            EDIValidationException next = current.nextException;
-            if (next != null) {
-                current = next;
-                continue;
-            }
-
-            if (current.nextException == null) {
-                current.nextException = ex;
-                return;
-            }
 
+        while (current.nextException != null) {
             current = current.nextException;
         }
+
+        current.nextException = ex;
     }
 }


Test patch:
diff --git a/src/test/java/io/xlate/edi/internal/stream/StaEDIStreamReaderTest.java b/src/test/java/io/xlate/edi/internal/stream/StaEDIStreamReaderTest.java
index d84e354..1e186ba 100644
--- a/src/test/java/io/xlate/edi/internal/stream/StaEDIStreamReaderTest.java
+++ b/src/test/java/io/xlate/edi/internal/stream/StaEDIStreamReaderTest.java
@@ -40,6 +40,7 @@ import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
 
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.w3c.dom.Document;
 import org.xml.sax.SAXException;
@@ -1524,4 +1525,54 @@ class StaEDIStreamReaderTest implements ConstantsTest {
         assertEquals("373", errorReferences.get(0).getReferencedType().getCode());
         assertEquals("373", errorReferences.get(1).getReferencedType().getCode());
     }
+
+    @Test
+    void testEmptySegmentAtLoopStartValidation() throws Exception {
+
+        EDIInputFactory factory = EDIInputFactory.newFactory();
+        Schema transSchema = SchemaFactory.newFactory().createSchema(getClass().getResourceAsStream("/EDIFACT/empty-segment-loop-schema.xml"));
+        factory.setProperty(EDIInputFactory.EDI_VALIDATE_CONTROL_STRUCTURE, true);
+        factory.setProperty(EDIInputFactory.EDI_VALIDATE_CONTROL_CODE_VALUES, false);
+
+        EDIStreamReader reader = factory.createEDIStreamReader(getClass().getResourceAsStream("/EDIFACT/empty-segment-loop-example.edi"));
+        String segmentName = null;
+
+        try {
+            while (reader.hasNext()) {
+                switch (reader.next()) {
+                case START_TRANSACTION:
+                    reader.setTransactionSchema(transSchema);
+                    break;
+
+                case START_SEGMENT:
+                    segmentName = reader.getText();
+                    break;
+
+                case END_SEGMENT:
+                    segmentName = null;
+                    break;
+
+                case ELEMENT_DATA:
+                    break;
+
+                case SEGMENT_ERROR:
+                case ELEMENT_DATA_ERROR:
+                case ELEMENT_OCCURRENCE_ERROR:
+                    String textOnError = reader.getText();
+                    Location loc = reader.getLocation();
+                    EDIStreamValidationError error = reader.getErrorType();
+
+                    // Fails printing:
+                    // java.lang.AssertionError: SEGMENT_ERROR: SEGMENT_EXCEEDS_MAXIMUM_USE (refCode=RCI, seg=null, elemPos=-1, compoPos=-1, textOnError=EQN)
+                    Assertions.fail(String.format("%s: %s (refCode=%s, textOnError=%s, loc=%s)",
+                            error.getCategory(), error, reader.getReferenceCode(), textOnError, loc));
+                    break;
+                default:
+                    break;
+                }
+            }
+        } finally {
+            reader.close();
+        }
+    }
 }
diff --git a/src/test/java/io/xlate/edi/internal/stream/validation/AlphaNumericValidatorTest.java b/src/test/java/io/xlate/edi/internal/stream/validation/AlphaNumericValidatorTest.java
index 189ea74..c22c35d 100644
--- a/src/test/java/io/xlate/edi/internal/stream/validation/AlphaNumericValidatorTest.java
+++ b/src/test/java/io/xlate/edi/internal/stream/validation/AlphaNumericValidatorTest.java
@@ -8,17 +8,20 @@ import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentMatchers;
 
 import io.xlate.edi.internal.stream.tokenization.CharacterSet;
 import io.xlate.edi.internal.stream.tokenization.Dialect;
 import io.xlate.edi.internal.stream.tokenization.DialectFactory;
 import io.xlate.edi.internal.stream.tokenization.EDIException;
 import io.xlate.edi.schema.EDISimpleType;
+import io.xlate.edi.stream.EDIStreamException;
 import io.xlate.edi.stream.EDIStreamValidationError;
 import io.xlate.edi.stream.EDIValidationException;
 
@@ -181,4 +184,20 @@ class AlphaNumericValidatorTest implements ValueSetTester {
         v.format(dialect, element, "TEST", output);
         assertEquals("TEST      ", output.toString());
     }
+
+    @Test
+    void testFormatValidValueIOException() throws Exception {
+        EDISimpleType element = mock(EDISimpleType.class);
+        when(element.getMinLength(anyString())).thenCallRealMethod();
+        when(element.getMaxLength(anyString())).thenCallRealMethod();
+        when(element.getValueSet(anyString())).thenCallRealMethod();
+
+        when(element.getMinLength()).thenReturn(10L);
+        when(element.getMaxLength()).thenReturn(10L);
+        ElementValidator v = AlphaNumericValidator.getInstance();
+        Appendable output = mock(Appendable.class);
+        when(output.append(ArgumentMatchers.anyChar())).thenThrow(IOException.class);
+        EDIStreamException thrown = assertThrows(EDIStreamException.class, () -> v.format(dialect, element, "TEST", output));
+        assertEquals(IOException.class, thrown.getCause().getClass());
+    }
 }
diff --git a/src/test/java/io/xlate/edi/stream/EDIValidationExceptionTest.java b/src/test/java/io/xlate/edi/stream/EDIValidationExceptionTest.java
new file mode 100644
index 0000000..0a3cae4
--- /dev/null
+++ b/src/test/java/io/xlate/edi/stream/EDIValidationExceptionTest.java
@@ -0,0 +1,42 @@
+package io.xlate.edi.stream;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import org.junit.jupiter.api.Test;
+
+class EDIValidationExceptionTest {
+
+    @Test
+    void testSetNextException() {
+        EDIValidationException[] exceptions = { new EDIValidationException(null, null, null, null),
+                                                new EDIValidationException(null, null, null, null),
+                                                new EDIValidationException(null, null, null, null),
+                                                new EDIValidationException(null, null, null, null),
+                                                new EDIValidationException(null, null, null, null)
+        };
+
+        EDIValidationException primary = new EDIValidationException(null, null, null, null);
+        primary.setNextException(exceptions[0]);
+        primary.setNextException(exceptions[1]);
+        primary.setNextException(exceptions[2]);
+        primary.setNextException(exceptions[3]);
+        primary.setNextException(exceptions[4]);
+
+        assertSame(exceptions[0], primary.getNextException());
+        assertSame(exceptions[1], primary.getNextException()
+                   .getNextException());
+        assertSame(exceptions[2], primary.getNextException()
+                   .getNextException()
+                   .getNextException());
+        assertSame(exceptions[3], primary.getNextException()
+                   .getNextException()
+                   .getNextException()
+                   .getNextException());
+        assertSame(exceptions[4], primary.getNextException()
+                   .getNextException()
+                   .getNextException()
+                   .getNextException()
+                   .getNextException());
+    }
+
+}
diff --git a/src/test/resources/EDIFACT/empty-segment-loop-example.edi b/src/test/resources/EDIFACT/empty-segment-loop-example.edi
new file mode 100644
index 0000000..f3bcd0a
--- /dev/null
+++ b/src/test/resources/EDIFACT/empty-segment-loop-example.edi
@@ -0,0 +1,17 @@
+UNA:+.?*'
+UNB+IATA:1+1A+ONIP+180817:1000+0001+++X'
+UNH+1+PNRGOV:11:1:IA+170818/1000/NT/6810'
+MSG+:22'
+ORG+1A:MUC'
+TVL+190818:1100:190818:1420+LPA+LIS+NT+6810'
+EQN+2'
+SRC'
+RCI+1A:JP9PVU::120418:1621+1S:MASGXD:O'
+DAT+700:120418:1750'
+ORG+1A:MUC+78237084:DFW1S4100+HDQ+1S:HDQ++ES+AASU'
+SRC'
+RCI+1A:NT3A3P::250418:1002'
+DAT+700:250418:1003'
+ORG+1A:MUC+78497650:TCINT08AB+LPA+NT:LPA+A+ES+JDSU'
+UNT+211+1'
+UNZ+1+0001'
diff --git a/src/test/resources/EDIFACT/empty-segment-loop-schema.xml b/src/test/resources/EDIFACT/empty-segment-loop-schema.xml
new file mode 100644
index 0000000..1801407
--- /dev/null
+++ b/src/test/resources/EDIFACT/empty-segment-loop-schema.xml
@@ -0,0 +1,83 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+
+<schema xmlns="http://xlate.io/EDISchema/v4">
+
+    <transaction>
+        <sequence>
+            <segment type="MSG" minOccurs="1" maxOccurs="1" />
+            <segment type="ORG" minOccurs="1" maxOccurs="1" />
+            <segment type="TVL" minOccurs="1" maxOccurs="1" />
+            <segment type="EQN" minOccurs="1" maxOccurs="1" />
+            <loop code="GR1_SRC" maxOccurs="999">
+                <sequence>
+                    <!-- Empty segment starts the looop -->
+                    <segment type="SRC" minOccurs="1" maxOccurs="1" />
+                    <segment type="RCI" minOccurs="1" maxOccurs="1" />
+                    <segment type="DAT" minOccurs="0" maxOccurs="1" />
+                    <segment type="ORG" minOccurs="1" maxOccurs="1" />
+                </sequence>
+             </loop>
+        </sequence>
+    </transaction>
+
+    <!-- To simplify, same as transaction -->
+    <implementation>
+        <sequence>
+            <segment type="MSG" />
+            <segment type="ORG" />
+            <segment type="TVL" />
+            <segment type="EQN" />
+            <loop code="GR1_SRC_CODE" type="GR1_SRC" >
+                <sequence>
+                    <segment type="SRC" />
+                    <segment type="RCI" />
+                    <segment type="DAT" />
+                    <segment type="ORG" />
+                </sequence>
+             </loop>
+        </sequence>
+    </implementation>
+
+    <!-- The empty segment -->
+    <segmentType name="SRC">
+        <sequence/>
+    </segmentType>
+
+    <!-- Don't mind the other data components -->
+    <segmentType name="MSG">
+        <sequence>
+            <any minOccurs="0" maxOccurs="10" />
+        </sequence>
+    </segmentType>
+
+    <segmentType name="ORG">
+        <sequence>
+            <any minOccurs="0" maxOccurs="10" />
+        </sequence>
+    </segmentType>
+
+    <segmentType name="TVL">
+        <sequence>
+            <any minOccurs="0" maxOccurs="10" />
+        </sequence>
+    </segmentType>
+
+    <segmentType name="EQN">
+        <sequence>
+            <any minOccurs="0" maxOccurs="10" />
+        </sequence>
+    </segmentType>
+
+    <segmentType name="RCI">
+        <sequence>
+            <any minOccurs="0" maxOccurs="10" />
+        </sequence>
+    </segmentType>
+
+    <segmentType name="DAT">
+        <sequence>
+            <any minOccurs="0" maxOccurs="10" />
+        </sequence>
+    </segmentType>
+
+</schema>