Skip to content

Implement XML Matrix decoding in OpcUaXmlDecoder#1763

Merged
kevinherron merged 4 commits into
eclipse-milo:mainfrom
LivingLikeKrillin:fix-xml-matrix-decode
Jun 5, 2026
Merged

Implement XML Matrix decoding in OpcUaXmlDecoder#1763
kevinherron merged 4 commits into
eclipse-milo:mainfrom
LivingLikeKrillin:fix-xml-matrix-decode

Conversation

@LivingLikeKrillin

@LivingLikeKrillin LivingLikeKrillin commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

OpcUaXmlDecoder's four Matrix decode methods (decodeMatrix, decodeEnumMatrix,
and the two decodeStructMatrix overloads) were stubs that returned Matrix.ofNull().
The encoder side (OpcUaXmlEncoder.encode*Matrix) is fully implemented, so a Matrix
encoded to XML produced well-formed output but did not round-trip — the value was
silently lost on decode.

This was raised in #1762, where the maintainer confirmed it was missed during the
1.0 OpcUaXmlEncoder work and gave the go-ahead (pointing to OPC 10000-6 §5.3).

Changes

  • decodeMatrix reads the <Dimensions> and <Elements> the encoder writes and
    rebuilds the Matrix with the requested OpcUaDataType.
  • decodeEnumMatrix reduces each element to Int32, and decodeStructMatrix decodes
    the self-describing ExtensionObject elements back into their structures (returning
    a Matrix of UaStructuredType) — both mirror OpcUaBinaryDecoder and keep the
    decode→encode round-trip intact.
  • Locate the <Dimensions>/<Elements> wrappers by element type, so indented XML
    (e.g. copied from a UANodeSet) also decodes.
  • Reject malformed Matrix XML with Bad_DecodingError instead of building an invalid
    Matrix, per OPC 10000-6 §5.3: dimensions whose product does not match the element
    count, non-positive dimensions, builtin elements whose XML tag name is not the
    requested type name, and (for decodeStructMatrix) ExtensionObjects that decode to
    a different structure type than the requested dataTypeId — the last resolves the
    codec for dataTypeId and validates against it, mirroring decodeStructArray.
  • Guard Matrix.toString() against the null-Matrix NullPointerException noted in
    the issue (ArrayUtil.getType on a null flatArray).

Tests

  • OpcUaXmlDecoderMatrixTest: round-trips the same MatrixArguments fixtures the
    encoder tests use across the builtin, enumerated, and structured types; asserts that
    struct matrices decode back to UaStructuredType; indented-XML and null-Matrix
    decode cases; plus the validation cases for malformed XML (mismatched/non-positive
    dimensions, wrong element name, unexpected struct type).
  • MatrixTest: toString() on a null Matrix no longer throws.

Fixes #1762


Developed with AI assistance (Anthropic Claude Opus 4.8); the human contributor
reviewed, verified, and tested all changes.

LivingLikeKrillin and others added 3 commits June 4, 2026 08:43
The four Matrix decode methods (decodeMatrix, decodeEnumMatrix, and the
two decodeStructMatrix overloads) were stubs that returned Matrix.ofNull(),
so a Matrix encoded by OpcUaXmlEncoder did not round-trip through the XML
encoding: the value was silently lost on decode.

Decode the <Dimensions> and <Elements> children written by the encoder.
decodeMatrix reads each element by its builtin type and rebuilds the Matrix
with the requested OpcUaDataType. decodeEnumMatrix reads each element as an
enum value and reduces it to Int32. decodeStructMatrix decodes the
self-describing ExtensionObject elements back into their structures, so it
returns a Matrix of UaStructuredType; all three mirror the reductions
OpcUaBinaryDecoder performs and keep the decode-encode round-trip intact.
The <Dimensions>/<Elements> wrappers are located by element type so the
decoder also accepts indented XML (e.g. from a UANodeSet).

Add OpcUaXmlDecoderMatrixTest, which round-trips the same fixtures the
encoder tests use across the builtin, enumerated, and structured types.

Fixes eclipse-milo#1762

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
Matrix.ofNull().toString() threw a NullPointerException because
ArrayUtil.getType was called with a null flatArray. Return early with a
flatArray=null rendering instead.

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
Cover malformed Matrix XML that should be rejected instead of
producing invalid Matrix values, including mismatched dimensions,
non-positive dimensions, wrong element names, and unexpected
structured element types.
@kevinherron

kevinherron commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

I pushed a small set of failing test cases in e86f87017 that capture behavior the XML Matrix decoder should handle before this lands. The tests document expected decoder behavior; the implementation still needs to be updated to satisfy them.

  1. Dimension/product validation

    OPC 10000-6 section 5.3.1.17 says all Matrix dimensions must be specified and greater than zero, and that inconsistent dimensions must cause the decoder to stop and raise Bad_DecodingError:

    https://reference.opcfoundation.org/specs/OPC-10000-6/5.3

    The added test uses dimensions [2, 2] with only one element. That should fail because a 2x2 Matrix requires exactly 4 flattened elements.

  2. Non-positive dimensions

    The same section says Matrix dimensions must be greater than zero. The added test uses a zero dimension and expects the decoder to reject it instead of producing a malformed Matrix.

  3. Element XML type-name validation

    OPC 10000-6 section 5.3.4 says array element names shall be the type name, and section 5.3.1.17 defines Matrix <Elements> as the flattened array contents.

    If the caller requests an Int32 Matrix, the entries under <Elements> should be <Int32> elements. The added test checks that an Int32 Matrix rejects a <String> element.

  4. Struct Matrix expected type

    decodeStructMatrix("Test", XVType.TYPE_ID) should not silently accept ExtensionObjects whose bodies decode to a different structure type, such as ThreeDVector.

    XML ExtensionObjects are self-describing, but for a non-subtyped struct field the dataTypeId argument is still the declared/expected structure type for that field. The added test encodes a ThreeDVector Matrix and tries to decode it as XVType, expecting the decoder to reject it.

The decoder trusted the XML structure and could build invalid Matrix values
from malformed input. Reject it instead, per OPC 10000-6 §5.3, covering the
cases added in the previous commit:

- dimensions whose product does not match the number of decoded elements
  (§5.3.1.17);
- non-positive dimensions (§5.3.1.17 requires every dimension > 0);
- a builtin element whose XML tag name is not the requested type name, e.g. a
  <String> element in an Int32 Matrix (§5.3.4);
- decodeStructMatrix now resolves the codec for the requested dataTypeId and
  rejects ExtensionObjects that decode to a different structure type, mirroring
  decodeStructArray.

Signed-off-by: Jooyoung Jung <livinglikekrillin@gmail.com>
@LivingLikeKrillin

Copy link
Copy Markdown
Contributor Author

Thanks, these probes were helpful — they map cleanly onto four validation gaps. I've pushed a follow-up commit (bc0d22885) that implements all four; the test cases you added now pass, and the existing round-trip tests stay green.

  • Dimension/product — after reading <Dimensions> and <Elements>, the decoder checks that the product of the dimensions equals the number of decoded elements and raises Bad_DecodingError otherwise (decodeMatrix and decodeEnumMatrix share the check).
  • Non-positive dimensions — each dimension is validated as > 0 while parsing <Dimensions>.
  • Element type namedecodeMatrix now requires each <Elements> child's tag name to equal the requested type name, so an Int32 Matrix rejects a <String> element instead of parsing its text as an Int32.
  • Struct Matrix type — you're right that the declared dataTypeId should be enforced. I'd originally left it out, reasoning the ExtensionObjects are self-describing, but rejecting an unexpected type is the safer contract for a non-subtyped field. decodeStructMatrix now resolves the codec for dataTypeId and validates each decoded structure against codec.getType(), mirroring decodeStructArray (the ExpandedNodeId overload resolves and throws on an unregistered namespace, as the binary decoder does).

I've also dropped the now-inaccurate "dataTypeId is not needed" note from the PR description.

@kevinherron kevinherron merged commit 3cdc86d into eclipse-milo:main Jun 5, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpcUaXmlDecoder returns a null Matrix for Matrix values (decode side of the XML Matrix mapping)

2 participants