Skip to content

Commit

Permalink
Fix validation of empty first component elements (#509)
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Edgar <[email protected]>
  • Loading branch information
MikeEdgar authored Oct 31, 2024
1 parent 36c7223 commit 2e650aa
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 10 deletions.
12 changes: 10 additions & 2 deletions src/main/java/io/xlate/edi/internal/stream/StaEDIStreamWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,7 @@ public boolean elementData(CharSequence text, boolean fromStream) {
dialect.elementData(elementHolder, location);

validator().ifPresent(validator -> {
if (!validator.validateElement(dialect, location, elementHolder, null)) {
if (!validator.validateElement(dialect, location, elementHolder, derivedComposite(validator), null)) {
reportElementErrors(validator, elementHolder);
}
});
Expand Down Expand Up @@ -960,6 +960,14 @@ public void segmentError(CharSequence token, EDIReference typeReference, EDIStre
}
}

private boolean derivedComposite(Validator validator) {
if (location.getComponentPosition() > -1) {
return false;
}

return validator.isComposite(dialect, location);
}

private void validate(Consumer<Validator> command) {
validator().ifPresent(validator -> {
errors.clear();
Expand Down Expand Up @@ -1005,7 +1013,7 @@ CharSequence validateElement(Runnable setupCommand, CharSequence data, Validator
errors.clear();
setupCommand.run();

if (!validator.validateElement(dialect, location, data, this.formattedElement)) {
if (!validator.validateElement(dialect, location, data, derivedComposite(validator), this.formattedElement)) {
reportElementErrors(validator, elementData);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ public boolean elementData(CharSequence text, boolean fromStream) {
location.incrementComponentPosition();
}

valid = validator.validateElement(dialect, location, text, null);
valid = validator.validateElement(dialect, location, text, derivedComposite, null);
typeReference = validator.getElementReference();
enqueueElementOccurrenceErrors(text, validator, valid);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,7 @@ public boolean isComposite(Dialect dialect, StaEDIStreamLocation position) {
return false;
}

public boolean validateElement(Dialect dialect, StaEDIStreamLocation position, CharSequence value, StringBuilder formattedValue) {
public boolean validateElement(Dialect dialect, StaEDIStreamLocation position, CharSequence value, boolean derivedComposite, StringBuilder formattedValue) {
if (!segmentExpected) {
return true;
}
Expand Down Expand Up @@ -1143,7 +1143,7 @@ public boolean validateElement(Dialect dialect, StaEDIStreamLocation position, C
}

if (componentIndex > -1) {
validateComponentElement(dialect, componentIndex, valueReceived);
validateComponentElement(dialect, componentIndex, valueReceived, derivedComposite);
} else {
// Validated in validCompositeOccurrences for received composites
validateImplUnusedElementBlank(this.element, this.implElement, valueReceived);
Expand All @@ -1162,7 +1162,7 @@ public boolean validateElement(Dialect dialect, StaEDIStreamLocation position, C
return elementErrors.isEmpty();
}

void validateComponentElement(Dialect dialect, int componentIndex, boolean valueReceived) {
void validateComponentElement(Dialect dialect, int componentIndex, boolean valueReceived, boolean derivedComposite) {
if (!element.isNodeType(EDIType.Type.COMPOSITE)) {
/*
* This element has components but is not defined as a composite
Expand All @@ -1177,7 +1177,7 @@ void validateComponentElement(Dialect dialect, int componentIndex, boolean value
String version = dialect.getTransactionVersionString();

if (componentIndex < element.getChildren(version).size()) {
if (valueReceived || componentIndex != 0 /* Derived component */) {
if (valueReceived || componentIndex != 0 /* Derived component */ || !derivedComposite) {
this.element = this.element.getChild(version, componentIndex);

if (isImplElementSelected()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.Set;
import java.util.logging.Logger;
import java.util.stream.Collectors;

import javax.xml.parsers.DocumentBuilder;
Expand Down Expand Up @@ -80,6 +81,7 @@
@SuppressWarnings({ "resource", "unused" })
class StaEDIStreamReaderTest implements ConstantsTest {

private static final Logger LOG = Logger.getLogger(StaEDIStreamReaderTest.class.getName());
private Set<EDIStreamEvent> possible = new HashSet<>();

public StaEDIStreamReaderTest() {
Expand Down Expand Up @@ -1907,6 +1909,7 @@ void testValidatorLookAhead_Issue122() throws Exception {
case SEGMENT_ERROR:
case ELEMENT_OCCURRENCE_ERROR:
case ELEMENT_DATA_ERROR:
LOG.warning(String.format("Unexpected error: %s, %s at %s", reader.getEventType(), reader.getErrorType(), reader.getLocation()));
unexpected.add(reader.getErrorType());
break;
default:
Expand All @@ -1919,7 +1922,57 @@ void testValidatorLookAhead_Issue122() throws Exception {
reader.close();
}

assertEquals(0, unexpected.size());
assertEquals(0, unexpected.size(), () -> unexpected.toString());
}

/**
* Original issue: https://github.com/xlate/staedi/issues/508
*/
@Test
void testMissingComponentTriggersError() throws Exception {
EDIInputFactory factory = EDIInputFactory.newFactory();
Schema transSchema = SchemaFactory.newFactory().createSchema(getClass().getResourceAsStream("/EDIFACT/issue122/BAPLIE-d95b.xml"));
EDIStreamReader reader = factory.createEDIStreamReader(new ByteArrayInputStream((""
+ "UNB+UNOA:2+SENDER+RECIPIENT+090304:1230+UNIQUEID1234+++++SHIPPINGLINE'\n"
+ "UNH+SENDER123+BAPLIE:D:95B:UN:SMDG20'\n"
+ "BGM++M1+9'\n"
+ "DTM+137:0903031447:201'\n"
+ "TDT+20+VOYAGENO123+++CARRIERID:172:20+++DLHV:103:ZZZ'\n"
+ "LOC+161+::92'\n" // LOC02-1 required and missing
+ "DTM+132'\n"
+ "UNT+7+SENDER123'\n"
+ "UNZ+1+UNIQUEID1234'").getBytes()));
List<Location> errorLocations = new ArrayList<>();
List<EDIStreamValidationError> errors = new ArrayList<>();

try {
while (reader.hasNext()) {
switch (reader.next()) {
case START_TRANSACTION:
reader.setTransactionSchema(transSchema);
break;
case SEGMENT_ERROR:
case ELEMENT_OCCURRENCE_ERROR:
case ELEMENT_DATA_ERROR:
errors.add(reader.getErrorType());
errorLocations.add(reader.getLocation().copy());
break;
default:
break;
}
}
} finally {
reader.close();
}

assertEquals(1, errors.size(), () -> errors + "; " + errorLocations.toString());
assertEquals(EDIStreamValidationError.REQUIRED_DATA_ELEMENT_MISSING, errors.get(0));

assertEquals(1, errorLocations.size());
assertEquals("LOC", errorLocations.get(0).getSegmentTag());
assertEquals(6, errorLocations.get(0).getSegmentPosition());
assertEquals(2, errorLocations.get(0).getElementPosition());
assertEquals(1, errorLocations.get(0).getComponentPosition());
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions src/test/resources/EDIFACT/issue122/BAPLIE-d95b.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2237,7 +2237,7 @@
</compositeType>
<compositeType name="CEC517">
<sequence>
<element type="DE3225"/>
<element type="DE3225" minOccurs="1"/>
<element type="DE1131"/>
<element type="DE3055"/>
<element type="DE3224"/>
Expand Down Expand Up @@ -2343,7 +2343,7 @@
<segmentType name="LOC">
<sequence>
<element type="DE3227" minOccurs="1"/>
<composite type="CEC517"/>
<composite type="CEC517" minOccurs="1"/>
<composite type="CEC519"/>
<composite type="CEC553"/>
<element type="DE5479"/>
Expand Down

0 comments on commit 2e650aa

Please sign in to comment.