Skip to content
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.

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

Fix validation of empty first component elements #509

Merged
merged 1 commit into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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