You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by sr...@apache.org on 2026/08/18 12:41:12 UTC
(plc4x) 01/02: fix(plc4j/spi): make the XML buffer able to read back its own writer's output
This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
commit 2ed860e80cbd96258984777d06f197cba91d90dc
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Tue Aug 18 14:39:01 2026 +0200
fix(plc4j/spi): make the XML buffer able to read back its own writer's output
The driver testsuite runner round-trips messages through the XML buffers,
which exposed several parse/serialize asymmetries that made any XML-driven
driver testsuite fail:
- reserved fields are serialized as plain <reserved> but parsed with the
qualified '<Type>.reservedN' name - the reader now accepts the
serializer's form
- complex array elements are serialized without a logical-name wrapper
(the element type provides the element), but the complex data reader
pushed one per element - array element reads now carry the
render-as-list marker and the complex reader skips the wrapper for them
- enum fields are serialized as <fieldName><EnumTypeName ...> where the
inner name is only known dynamically - the enum field reader now reads
the inner element with a wildcard name
- padding fields are serialized under an explicit 'padding' context that
the reader didn't set
The testsuite runner's byte buffers also lacked the standard
unsigned-binary/twos-complement/IEEE754 encodings every driver codec
configures, so generated (de)serializers failed on fields without
explicit per-field options.
Byte-based parsing is unaffected: contexts and logical names are no-ops
there.
---
.../java/spi/buffers/xmlbased/ReadBufferXmlBased.java | 14 ++++++++++++--
.../spi/fields/data/reader/DataReaderComplexDefault.java | 6 ++++++
.../java/spi/fields/fields/reader/FieldReaderArray.java | 16 +++++++++++++---
.../java/spi/fields/fields/reader/FieldReaderEnum.java | 5 ++++-
.../spi/fields/fields/reader/FieldReaderPadding.java | 5 +++--
.../internal/handlers/IncomingPlcMessageHandler.java | 9 ++++++++-
.../internal/handlers/OutgoingPlcMessageHandler.java | 10 +++++++++-
7 files changed, 55 insertions(+), 10 deletions(-)
diff --git a/plc4j/spi/buffers/xml/src/main/java/org/apache/plc4x/java/spi/buffers/xmlbased/ReadBufferXmlBased.java b/plc4j/spi/buffers/xml/src/main/java/org/apache/plc4x/java/spi/buffers/xmlbased/ReadBufferXmlBased.java
index b18c19896b..6876646590 100644
--- a/plc4j/spi/buffers/xml/src/main/java/org/apache/plc4x/java/spi/buffers/xmlbased/ReadBufferXmlBased.java
+++ b/plc4j/spi/buffers/xml/src/main/java/org/apache/plc4x/java/spi/buffers/xmlbased/ReadBufferXmlBased.java
@@ -287,12 +287,22 @@ public class ReadBufferXmlBased extends AbstractBuffer implements ReadBuffer, Xm
private void validateStartElement(StartElement startElement, String name, String dataType, int numBits) throws BufferException {
//name = sanitizeLogicalName(name);
- if (!startElement.getName().getLocalPart().equals(name)) {
- throw new BufferException(String.format("unexpected element '%s'. Expected '%s'", startElement.getName().getLocalPart(), name));
+ String elementName = startElement.getName().getLocalPart();
+ // "*" matches any element name: used for elements whose serialized name is only known
+ // dynamically (e.g. enum fields are written as <fieldName><EnumTypeName ...>).
+ if (!"*".equals(name) && !elementName.equals(name) && !isReservedFieldName(name, elementName)) {
+ throw new BufferException(String.format("unexpected element '%s'. Expected '%s'", elementName, name));
}
validateAttr(startElement.getAttributes(), name, dataType, numBits);
}
+ private boolean isReservedFieldName(String expectedName, String elementName) {
+ // The generated parsers qualify reserved fields as "<Type>.reserved<N>" while the
+ // generated serializers write them plainly as "reserved" - accept the serializer's
+ // form so XML written by WriteBufferXmlBased can be read back.
+ return "reserved".equals(elementName) && expectedName.matches("(.*\\.)?reserved\\d*");
+ }
+
private void validateAttr(Iterator<Attribute> attr, String name, String dataType, int numBits) throws BufferException {
boolean dataTypeValidated = false;
boolean numBitsValidate = false;
diff --git a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/data/reader/DataReaderComplexDefault.java b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/data/reader/DataReaderComplexDefault.java
index b0673f0c83..6bcadc8f66 100644
--- a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/data/reader/DataReaderComplexDefault.java
+++ b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/data/reader/DataReaderComplexDefault.java
@@ -50,6 +50,12 @@ public class DataReaderComplexDefault<T> implements DataReaderComplex<T> {
}
public T read(ComplexTypeSupplier<T> complexSupplier, WithOption... options) throws BufferException {
+ // Array elements carry the render-as-list marker: the serializer writes them without a
+ // logical-name wrapper (the element type's own serialize provides the element), so
+ // skip the wrapper context here as well to keep parse and serialize symmetric.
+ if (WithOption.extractRenderAsList(options).orElse(false)) {
+ return complexSupplier.get();
+ }
readBuffer.pushContext(options);
final T t = complexSupplier.get();
readBuffer.popContext(options);
diff --git a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderArray.java b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderArray.java
index 9f454bb67f..df9acfeb52 100644
--- a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderArray.java
+++ b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderArray.java
@@ -64,7 +64,7 @@ public class FieldReaderArray<T> implements FieldCommons {
// Make some variables available that would be otherwise challenging to forward.
ThreadLocalHelper.curItemThreadLocal.set(curItem);
ThreadLocalHelper.lastItemThreadLocal.set(curItem == itemCount - 1);
- result.add(dataReader.read(options));
+ result.add(dataReader.read(listElementOptions(options)));
}
dataReader.popContext(options);
LOGGER.debug("done reading field {}", getName(options));
@@ -83,7 +83,7 @@ public class FieldReaderArray<T> implements FieldCommons {
LOGGER.debug("start reading at pos {} while < {}", startPos, stopPosition);
while (dataReader.getPositionInBits() < stopPosition) {
numberOfElements++;
- T element = dataReader.read(options);
+ T element = dataReader.read(listElementOptions(options));
LOGGER.debug("Read element[{}] {}", numberOfElements, element);
result.add(element);
}
@@ -99,11 +99,21 @@ public class FieldReaderArray<T> implements FieldCommons {
dataReader.pushContext(options);
List<T> result = new ArrayList<>();
while (!termination.get()) {
- result.add(dataReader.read(options));
+ result.add(dataReader.read(listElementOptions(options)));
}
dataReader.popContext(options);
LOGGER.debug("done reading field {}", getName(options));
return result;
}
+ /**
+ * Marks an element read as part of a list: complex element types are serialized without a
+ * logical-name wrapper inside their list context, so the complex data reader skips the
+ * wrapper for reads carrying this marker (simple data readers ignore it).
+ */
+ private WithOption[] listElementOptions(WithOption[] options) {
+ // AddOptions prepends the new options, so the marker takes precedence.
+ return WithOption.AddOptions(options, WithOption.WithRenderAsList(true));
+ }
+
}
diff --git a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderEnum.java b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderEnum.java
index eb4f536028..f4a6617cdc 100644
--- a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderEnum.java
+++ b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderEnum.java
@@ -33,7 +33,10 @@ public class FieldReaderEnum<T> implements FieldCommons {
public T readEnumField(DataReader<T> dataReader, WithOption... options) throws BufferException {
LOGGER.debug("reading field {}", getName(options));
dataReader.pushContext(WithOption.AddOptions(options, WithOption.WithRenderAsList(true)));
- T result = dataReader.read(options);
+ // The serializer names the inner element after the enum type (see FieldWriterSimpleEnum),
+ // which isn't statically known here - read with the wildcard name so name-validating
+ // buffers (XML) accept it.
+ T result = dataReader.read(WithOption.AddOptions(options, WithOption.WithName("*")));
dataReader.popContext(WithOption.AddOptions(options, WithOption.WithRenderAsList(true)));
return result;
}
diff --git a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderPadding.java b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderPadding.java
index b77ebe93ba..0338a506b3 100644
--- a/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderPadding.java
+++ b/plc4j/spi/fields/src/main/java/org/apache/plc4x/java/spi/fields/fields/reader/FieldReaderPadding.java
@@ -32,7 +32,8 @@ public class FieldReaderPadding<T> implements FieldCommons {
public void readPaddingField(DataReader<T> dataReader, int timesPadding, WithOption... options) throws BufferException {
LOGGER.debug("reading field padding");
- dataReader.pushContext(WithOption.AddOptions(options, WithOption.WithRenderAsList(true)));
+ // Mirror FieldWriterPadding, which names the context "padding" explicitly.
+ dataReader.pushContext(WithOption.AddOptions(options, WithOption.WithRenderAsList(true), WithOption.WithName("padding")));
while (timesPadding-- > 0) {
// Just read the padding data and ignore it
try {
@@ -44,7 +45,7 @@ public class FieldReaderPadding<T> implements FieldCommons {
// last field item, then the packet might end here.
}
}
- dataReader.popContext(WithOption.AddOptions(options, WithOption.WithRenderAsList(true)));
+ dataReader.popContext(WithOption.AddOptions(options, WithOption.WithRenderAsList(true), WithOption.WithName("padding")));
}
}
diff --git a/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/java/utils/testutils/driver/internal/handlers/IncomingPlcMessageHandler.java b/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/java/utils/testutils/driver/internal/handlers/IncomingPlcMessageHandler.java
index b90f9826f6..099c62e08a 100644
--- a/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/java/utils/testutils/driver/internal/handlers/IncomingPlcMessageHandler.java
+++ b/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/java/utils/testutils/driver/internal/handlers/IncomingPlcMessageHandler.java
@@ -20,6 +20,7 @@ package org.apache.plc4x.java.utils.testutils.driver.internal.handlers;
import org.apache.plc4x.java.spi.buffers.api.Message;
import org.apache.plc4x.java.spi.buffers.api.ReadBuffer;
+import org.apache.plc4x.java.spi.buffers.api.WithOption;
import org.apache.plc4x.java.spi.buffers.api.exceptions.BufferException;
import org.apache.plc4x.java.spi.buffers.bytebased.WriteBufferByteBased;
import org.apache.plc4x.java.spi.buffers.xmlbased.ReadBufferXmlBased;
@@ -129,7 +130,13 @@ public class IncomingPlcMessageHandler {
}
try {
- WriteBufferByteBased writeBuffer = new WriteBufferByteBased(new byte[message.getLengthInBytes()]);
+ // The default integer/float encodings match what every driver codec configures;
+ // generated serializers (e.g. S7) fail without them on fields that don't pass
+ // explicit per-field options.
+ WriteBufferByteBased writeBuffer = new WriteBufferByteBased(new byte[message.getLengthInBytes()],
+ WithOption.WithUnsignedIntegerEncoding("unsigned-binary"),
+ WithOption.WithSignedIntegerEncoding("twos-complement"),
+ WithOption.WithFloatEncoding("IEEE754"));
writeBuffer.writeMessage(message);
byte[] bytes = writeBuffer.getBytes();
LOGGER.info("Sending serialized message: {}", StaticHelper.ENCODE_HEX(bytes));
diff --git a/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/java/utils/testutils/driver/internal/handlers/OutgoingPlcMessageHandler.java b/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/java/utils/testutils/driver/internal/handlers/OutgoingPlcMessageHandler.java
index bab2f21fea..c54f7b50e8 100644
--- a/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/java/utils/testutils/driver/internal/handlers/OutgoingPlcMessageHandler.java
+++ b/plc4j/utils/test-utils/src/main/java/org/apache/plc4x/java/utils/testutils/driver/internal/handlers/OutgoingPlcMessageHandler.java
@@ -20,6 +20,7 @@ package org.apache.plc4x.java.utils.testutils.driver.internal.handlers;
import org.apache.plc4x.java.spi.buffers.api.Message;
import org.apache.plc4x.java.spi.buffers.api.ReadBuffer;
+import org.apache.plc4x.java.spi.buffers.api.WithOption;
import org.apache.plc4x.java.spi.buffers.api.exceptions.BufferException;
import org.apache.plc4x.java.spi.buffers.bytebased.ReadBufferByteBased;
import org.apache.plc4x.java.spi.buffers.bytebased.WithByteBasedOption;
@@ -115,7 +116,14 @@ public class OutgoingPlcMessageHandler {
.findFirst()
.orElseThrow(() -> new RuntimeException("No staticParse method found in class " + className));
- ReadBufferByteBased readBuffer = new ReadBufferByteBased(actualBytes, WithByteBasedOption.WithByteOrder(byteOrder));
+ // The default integer/float encodings match what every driver codec configures;
+ // generated parsers (e.g. S7) fail without them on fields that don't pass
+ // explicit per-field options.
+ ReadBufferByteBased readBuffer = new ReadBufferByteBased(actualBytes,
+ WithOption.WithUnsignedIntegerEncoding("unsigned-binary"),
+ WithOption.WithSignedIntegerEncoding("twos-complement"),
+ WithOption.WithFloatEncoding("IEEE754"),
+ WithByteBasedOption.WithByteOrder(byteOrder));
Element parserArgumentsElement = referenceXml.element(new QName("parser-arguments"));