diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs index 0cae6ccb..a5409aba 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs @@ -1,5 +1,6 @@ using System.Globalization; using System.Runtime.InteropServices; +using System.Text; using System.Xml; using System.Xml.Linq; @@ -60,6 +61,28 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; /// /// /// +/// The read is forward-only. A /sample chunk is parsed once and thrown away, +/// so it is read straight off an rather than through an +/// : on the pump's hot path the tree was pure garbage — one +/// XElement plus its attributes and text nodes per observation, per chunk, per +/// second — and nothing here needs a second look at a node it has already passed. The +/// extraction is unchanged; the two rules that keep it honest are that whitespace-only text +/// nodes are dropped exactly as dropped them (see +/// ), and that an observation's slot in the result list is +/// reserved before its subtree is read so observations still arrive in document +/// order. The one place a tree is still built is the cold non-MTConnectStreams root +/// below, where the Agent's own error text has to be lifted out of a document this parser +/// is otherwise finished with. +/// +/// +/// One thing did change, and only for a document that is wrong twice over. A missing +/// <Header> used to be reported ahead of a malformed observation, because the +/// tree could be interrogated in whatever order suited; forward-only, the header is not +/// known to be missing until the root's children have run out, so the observation is +/// reported first. Both are , and nothing that is wrong in +/// only one way reports differently than it did. +/// +/// /// Failure posture. A malformed document, a non-MTConnectStreams root (an Agent /// MTConnectError served under HTTP 200 included), an unusable header, or an /// observation missing its dataItemId/timestamp all throw @@ -87,10 +110,32 @@ internal static class MTConnectStreamsParser private const string HeaderOwner = "the
"; /// - /// The three elements whose element children are observations. A ComponentStream may carry - /// any subset of them, including none. + /// How the whole document is read. Two settings are load-bearing rather than incidental. /// - private static readonly string[] ObservationContainers = ["Samples", "Events", ConditionContainer]; + /// + /// + /// reproduces + /// 's default. XDocument.Load/Parse without + /// sets exactly this, so a whitespace-only + /// text node never became an XText and never reached XElement.Value. + /// Leaving it at the reader's own default would silently start folding indentation into + /// observation values that contain element children — "1 2" where the tree read + /// "12". + /// + /// + /// A DTD is prohibited because a streams document is untrusted network input and + /// entity expansion is not a feature this parser has any use for. This too matches what + /// defaulted to. + /// + /// + private static readonly XmlReaderSettings ReaderSettings = new() + { + IgnoreWhitespace = true, + DtdProcessing = DtdProcessing.Prohibit, + + // The caller owns the stream it handed in; disposing the reader must not close it. + CloseInput = false + }; /// Parses a /current response, or one /sample chunk, held as a string. /// The raw MTConnectStreams XML document. @@ -107,19 +152,12 @@ internal static class MTConnectStreamsParser "MTConnect streams response was empty; expected an MTConnectStreams XML document."); } - XDocument document; - try - { - // A chunk lifted out of a multipart frame can carry a byte-order mark or the framing's - // trailing line break; XmlReader rejects either ahead of the XML declaration. - document = XDocument.Parse(xml.Trim('\uFEFF', ' ', '\t', '\r', '\n')); - } - catch (XmlException ex) - { - throw new InvalidDataException($"MTConnect streams response is not well-formed XML: {ex.Message}", ex); - } + // A chunk lifted out of a multipart frame can carry a byte-order mark or the framing's + // trailing line break; XmlReader rejects either ahead of the XML declaration. + using var text = new StringReader(xml.Trim('\uFEFF', ' ', '\t', '\r', '\n')); + using var reader = XmlReader.Create(text, ReaderSettings); - return Build(document); + return Read(reader); } /// @@ -128,7 +166,7 @@ internal static class MTConnectStreamsParser /// /// The /sample hot path used to materialise every chunk twice — once as a decoded /// string, once again as the trimmed copy handed to the parser — for a document that is read - /// once and thrown away. Parsing from the bytes drops both copies; + /// once and thrown away. Parsing from the bytes drops both copies; /// does its own UTF-8 decoding off the stream. The framing tolerance the string overload's /// Trim provided is preserved by , which slices rather than /// copies. @@ -156,17 +194,9 @@ internal static class MTConnectStreamsParser { ArgumentNullException.ThrowIfNull(stream); - XDocument document; - try - { - document = XDocument.Load(stream); - } - catch (XmlException ex) - { - throw new InvalidDataException($"MTConnect streams response is not well-formed XML: {ex.Message}", ex); - } + using var reader = XmlReader.Create(stream, ReaderSettings); - return Build(document); + return Read(reader); } /// @@ -218,57 +248,172 @@ internal static class MTConnectStreamsParser ? new MemoryStream(segment.Array, segment.Offset, segment.Count, writable: false) : new MemoryStream(xml.ToArray(), writable: false); - private static MTConnectStreamsResult Build(XDocument document) + /// + /// Reads the whole document, translating the reader's own well-formedness complaints into + /// this parser's failure family. A forward-only read raises them wherever it happens to + /// reach the damage rather than all at once up front, so the wrapper spans the entire walk. + /// + private static MTConnectStreamsResult Read(XmlReader reader) { - var root = document.Root - ?? throw new InvalidDataException("MTConnect streams response has no root element."); - - if (!MTConnectXml.IsNamed(root, "MTConnectStreams")) + try { - throw new InvalidDataException(MTConnectXml.DescribeUnexpectedRoot(root, "MTConnectStreams", Subject)); + return ReadDocument(reader); + } + catch (XmlException ex) + { + throw new InvalidDataException($"MTConnect streams response is not well-formed XML: {ex.Message}", ex); } - - var header = MTConnectXml.ChildrenNamed(root, "Header").FirstOrDefault() - ?? throw new InvalidDataException( - "MTConnect streams response has no
; without it the sequence bookkeeping the sample pump runs on is unknowable."); - - return new MTConnectStreamsResult( - MTConnectXml.RequiredLongAttribute(header, "instanceId", HeaderOwner, Subject), - MTConnectXml.RequiredLongAttribute(header, "nextSequence", HeaderOwner, Subject), - MTConnectXml.RequiredLongAttribute(header, "firstSequence", HeaderOwner, Subject), - ReadObservations(root)); } - /// - /// Collects every observation in the document, in Agent-supplied order. Containers are found - /// by descending the whole <Streams> subtree rather than by walking a fixed - /// DeviceStream → ComponentStream chain, so a vendor's extra nesting level cannot silently - /// hide a device's observations. - /// - private static IReadOnlyList ReadObservations(XElement root) + private static MTConnectStreamsResult ReadDocument(XmlReader reader) { - var observations = new List(); - - foreach (var container in root.Descendants().Where(IsObservationContainer)) + while (reader.Read() && reader.NodeType != XmlNodeType.Element) { - var isCondition = MTConnectXml.IsNamed(container, ConditionContainer); - foreach (var element in container.Elements()) + // Skip the XML declaration, comments and processing instructions ahead of the root. + } + + if (reader.NodeType != XmlNodeType.Element) + { + throw new InvalidDataException("MTConnect streams response has no root element."); + } + + if (!MTConnectXml.IsNamed(reader, "MTConnectStreams")) + { + // The cold path, and the one place a tree is still built: an MTConnectError arrives in + // exactly this shape (Agents serve one under HTTP 200), and its elements — the + // Agent's own account of what it objected to — are the most useful thing in the whole + // response. Materialising the root is how the shared extraction can reach them. + throw new InvalidDataException( + MTConnectXml.DescribeUnexpectedRoot((XElement)XNode.ReadFrom(reader), "MTConnectStreams", Subject)); + } + + var observations = new List(); + HeaderSequences? header = null; + + if (!reader.IsEmptyElement) + { + while (reader.Read() && reader.NodeType != XmlNodeType.EndElement) { - observations.Add(ReadObservation(element, isCondition)); + if (reader.NodeType != XmlNodeType.Element) + { + continue; + } + + // The
is a direct child of the root, and the first one wins — but it is + // still walked like any other element rather than skipped, so a container nested + // inside it yields its observations exactly as the whole-subtree descent did. + if (header is null && MTConnectXml.IsNamed(reader, "Header")) + { + header = ReadHeader(reader); + } + + ReadElement(reader, ContainerKind.None, observations, textSink: null); } } - return observations; + var sequences = header ?? throw new InvalidDataException( + "MTConnect streams response has no
; without it the sequence bookkeeping the sample pump runs on is unknowable."); + + return new MTConnectStreamsResult( + sequences.InstanceId, + sequences.NextSequence, + sequences.FirstSequence, + observations); } - private static MTConnectObservation ReadObservation(XElement element, bool isCondition) - { - var dataItemId = MTConnectXml.RequiredAttribute(element, "dataItemId", $"Observation <{element.Name.LocalName}>", Subject); + /// + /// The three sequence numbers the sample pump runs on, read off the <Header>'s + /// start tag. Read eagerly, where the header is met, so an unusable header is still reported + /// ahead of anything the <Streams> that follows it might also be wrong about. + /// + private static HeaderSequences ReadHeader(XmlReader reader) => + new( + MTConnectXml.RequiredLongAttribute(reader, "instanceId", HeaderOwner, Subject), + MTConnectXml.RequiredLongAttribute(reader, "nextSequence", HeaderOwner, Subject), + MTConnectXml.RequiredLongAttribute(reader, "firstSequence", HeaderOwner, Subject)); - return new MTConnectObservation( - dataItemId, - isCondition ? ConditionState(element) : element.Value.Trim(), - ReadTimestampUtc(element, dataItemId), + /// + /// Consumes the element the reader is positioned on, collecting any observation it is and + /// any the rest of its subtree holds. Every element is descended into — observations are + /// found by their parent being one of the three containers, not by walking a fixed + /// DeviceStream → ComponentStream chain, so a vendor's extra nesting level cannot silently + /// hide a device's observations. + /// + /// + /// Cursor contract: on return the reader sits on the element's + /// , or on the element itself when it was written empty + /// — either way the caller's next lands on whatever follows. + /// Every recursion below relies on that, which is why the children are walked by hand + /// rather than through Skip/ReadSubtree. + /// + /// A reader positioned on the element's start tag. + /// What the enclosing element is, which decides whether this one is an observation. + /// Collects the observations found, in document order. + /// + /// The text accumulator of the nearest enclosing observation whose value is its text, or + /// null when there is none. Text found anywhere below an observation belongs to it — + /// that is what XElement.Value meant. + /// + private static void ReadElement( + XmlReader reader, + ContainerKind parent, + List observations, + ValueSink? textSink) + { + var isObservation = parent != ContainerKind.None; + var isCondition = parent == ContainerKind.Condition; + var kind = ContainerKindOf(reader); + + var slot = -1; + string? dataItemId = null; + var timestampUtc = default(DateTime); + string? conditionState = null; + + if (isObservation) + { + var name = reader.LocalName; + dataItemId = MTConnectXml.RequiredAttribute(reader, "dataItemId", $"Observation <{name}>", Subject); + timestampUtc = ReadTimestampUtc(reader, dataItemId); + conditionState = isCondition ? ConditionState(name) : null; + + // The slot is reserved BEFORE the subtree is read, and filled after: an observation is + // only complete once its text and its child-element flag are known, but appending it + // then would report a nested observation ahead of the one enclosing it. The tree walk + // this replaced was document-order, and callers key snapshots off that order. + slot = observations.Count; + observations.Add(null!); + } + + // A CONDITION's value is its element NAME, so nothing below it is collected for it — but an + // enclosing observation's text still is, hence the ancestor sink rather than none at all. + var sink = isObservation && !isCondition ? new ValueSink() : textSink; + + var hasElements = !reader.IsEmptyElement && ReadChildren(reader, kind, observations, sink); + + if (!isObservation) + { + return; + } + + string value; + if (isCondition) + { + value = conditionState!; + } + else + { + var raw = sink!.Value; + + // An observation nested inside another one contributes its text to the outer one too, + // which is what concatenating every descendant text node amounted to. + textSink?.Append(raw); + value = raw.Trim(); + } + + observations[slot] = new MTConnectObservation( + dataItemId!, + value, + timestampUtc, // A DATA_SET/TABLE observation keeps its content in children, so the // text read above concatenates them into nonsense ("12" for two entries). Flagged here @@ -276,7 +421,49 @@ internal static class MTConnectStreamsParser // MTConnectObservation.IsStructured. Never true for a CONDITION: its value comes from // the element NAME, so child content cannot corrupt it, and flagging one would throw // away a perfectly well-determined state. - IsStructured: !isCondition && element.HasElements); + IsStructured: !isCondition && hasElements); + } + + /// + /// Walks a non-empty element's children through its , + /// reporting whether any of them was an element. + /// + private static bool ReadChildren( + XmlReader reader, + ContainerKind kind, + List observations, + ValueSink? sink) + { + var hasElements = false; + + while (reader.Read()) + { + switch (reader.NodeType) + { + case XmlNodeType.Element: + hasElements = true; + ReadElement(reader, kind, observations, sink); + + break; + + // Deliberately not XmlNodeType.Whitespace: ReaderSettings suppresses it, and were it + // ever surfaced it would still not belong in the value — the tree this replaced + // dropped whitespace-only nodes on exactly the same setting. + case XmlNodeType.Text: + case XmlNodeType.CDATA: + case XmlNodeType.SignificantWhitespace: + sink?.Append(reader.Value); + + break; + + case XmlNodeType.EndElement: + return hasElements; + } + } + + // Unreachable on any input the reader accepts: running out of document inside an open + // element is a well-formedness error, which the reader raises from Read() itself. + return hasElements; } /// @@ -285,16 +472,12 @@ internal static class MTConnectStreamsParser /// token to recognize; every other state is reported as-named (Normal, Warning, /// Fault, and any vendor state). /// - private static string ConditionState(XElement element) - { - var state = element.Name.LocalName; + private static string ConditionState(string localName) => + string.Equals(localName, "Unavailable", StringComparison.OrdinalIgnoreCase) ? UnavailableSentinel : localName; - return string.Equals(state, "Unavailable", StringComparison.OrdinalIgnoreCase) ? UnavailableSentinel : state; - } - - private static DateTime ReadTimestampUtc(XElement element, string dataItemId) + private static DateTime ReadTimestampUtc(XmlReader reader, string dataItemId) { - var raw = MTConnectXml.RequiredAttribute(element, "timestamp", $"Observation '{dataItemId}'", Subject); + var raw = MTConnectXml.RequiredAttribute(reader, "timestamp", $"Observation '{dataItemId}'", Subject); if (!DateTime.TryParse( raw, @@ -311,7 +494,69 @@ internal static class MTConnectStreamsParser return DateTime.SpecifyKind(parsed, DateTimeKind.Utc); } - private static bool IsObservationContainer(XElement element) => - ObservationContainers.Any(name => MTConnectXml.IsNamed(element, name)); + /// + /// Which of the three observation containers — if any — the element the reader is positioned + /// on is. A ComponentStream may carry any subset of them, including none. + /// + private static ContainerKind ContainerKindOf(XmlReader reader) + { + if (MTConnectXml.IsNamed(reader, ConditionContainer)) + { + return ContainerKind.Condition; + } + return MTConnectXml.IsNamed(reader, "Samples") || MTConnectXml.IsNamed(reader, "Events") + ? ContainerKind.Values + : ContainerKind.None; + } + + /// What an element's children are: nothing special, valued observations, or conditions. + private enum ContainerKind + { + /// Not an observation container; its element children are just more structure. + None, + + /// <Samples> or <Events> — children take their value from their text. + Values, + + /// <Condition> — children take their value from their element NAME. + Condition + } + + /// The three sequence numbers read off a <Header>. + private readonly record struct HeaderSequences(long InstanceId, long NextSequence, long FirstSequence); + + /// + /// Accumulates one observation's text across however many nodes the parser hands it, without + /// a for the single-text-node shape every ordinary scalar + /// observation has — which is the shape this parser sees thousands of times a second. + /// + private sealed class ValueSink + { + private string? _single; + private StringBuilder? _builder; + + /// Everything appended so far, untrimmed — XElement.Value's concatenation. + public string Value => _builder?.ToString() ?? _single ?? string.Empty; + + public void Append(string text) + { + if (_builder is not null) + { + _builder.Append(text); + + return; + } + + if (_single is null) + { + _single = text; + + return; + } + + _builder = new StringBuilder(_single).Append(text); + _single = null; + } + } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs index d43c4f65..a8467fd2 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectXml.cs @@ -1,4 +1,5 @@ using System.Globalization; +using System.Xml; using System.Xml.Linq; namespace ZB.MOM.WW.OtOpcUa.Driver.MTConnect; @@ -33,6 +34,23 @@ internal static class MTConnectXml public static bool IsNamed(XElement element, string localName) => string.Equals(element.Name.LocalName, localName, StringComparison.Ordinal); + /// + /// Does the element the reader is positioned on have the given local name, whatever its + /// namespace? + /// + /// + /// reads its documents through a forward-only + /// rather than a materialised tree — one sample chunk is read once + /// and thrown away, so building an for it is pure waste. The reader + /// overloads here exist so that going forward-only did not fork the two silent rules in the + /// type-level remarks: both flavours match on the local name, read attributes unqualified, + /// and produce the same error text. + /// + /// A reader positioned on an element. + /// The unqualified name to match. + public static bool IsNamed(XmlReader reader, string localName) => + string.Equals(reader.LocalName, localName, StringComparison.Ordinal); + /// Direct element children of with the given local name. /// The element whose children to filter. /// The unqualified name to match. @@ -52,19 +70,46 @@ internal static class MTConnectXml return string.IsNullOrWhiteSpace(value) ? null : value; } + /// + /// Reads an unqualified attribute off the element the reader is positioned on, normalizing a + /// present-but-empty value to null. See for + /// why the reader overloads exist. + /// + /// + /// matches on the attribute's qualified + /// name, so a prefixed x:dataItemId does not answer to dataItemId — exactly + /// what rule 2 in the type-level remarks requires, and what + /// does for a namespace-free . + /// + /// A reader positioned on an element. + /// The unqualified attribute name. + public static string? OptionalAttribute(XmlReader reader, string name) + { + var value = reader.GetAttribute(name); + + return string.IsNullOrWhiteSpace(value) ? null : value; + } + /// Reads a required unqualified attribute, or throws naming the owner and the attribute. /// The element carrying the attribute. /// The unqualified attribute name. /// How to describe the element in the error message. /// How to describe the document in the error message (e.g. "/probe response"). /// The attribute is absent or blank. - public static string RequiredAttribute(XElement element, string name, string owner, string subject) - { - var value = OptionalAttribute(element, name); + public static string RequiredAttribute(XElement element, string name, string owner, string subject) => + OptionalAttribute(element, name) ?? throw MissingAttribute(name, owner, subject); - return value ?? throw new InvalidDataException( - $"MTConnect {subject} is malformed: {owner} has no '{name}' attribute."); - } + /// + /// Reads a required unqualified attribute off the element the reader is positioned on, or + /// throws naming the owner and the attribute. + /// + /// A reader positioned on an element. + /// The unqualified attribute name. + /// How to describe the element in the error message. + /// How to describe the document in the error message. + /// The attribute is absent or blank. + public static string RequiredAttribute(XmlReader reader, string name, string owner, string subject) => + OptionalAttribute(reader, name) ?? throw MissingAttribute(name, owner, subject); /// Reads an optional unqualified attribute as an . /// The element carrying the attribute. @@ -95,10 +140,28 @@ internal static class MTConnectXml /// How to describe the element in the error message. /// How to describe the document in the error message. /// The attribute is absent, blank, or not an integer. - public static long RequiredLongAttribute(XElement element, string name, string owner, string subject) - { - var raw = RequiredAttribute(element, name, owner, subject); + public static long RequiredLongAttribute(XElement element, string name, string owner, string subject) => + AsLong(RequiredAttribute(element, name, owner, subject), name, owner, subject); + /// + /// Reads a required unqualified attribute off the element the reader is positioned on, as a + /// . + /// + /// A reader positioned on an element. + /// The unqualified attribute name. + /// How to describe the element in the error message. + /// How to describe the document in the error message. + /// The attribute is absent, blank, or not an integer. + public static long RequiredLongAttribute(XmlReader reader, string name, string owner, string subject) => + AsLong(RequiredAttribute(reader, name, owner, subject), name, owner, subject); + + /// The one spelling of "that attribute is missing", shared by both flavours. + private static InvalidDataException MissingAttribute(string name, string owner, string subject) => + new($"MTConnect {subject} is malformed: {owner} has no '{name}' attribute."); + + /// The one spelling of "that attribute is not a number", shared by both flavours. + private static long AsLong(string raw, string name, string owner, string subject) + { if (!long.TryParse(raw, NumberStyles.Integer, CultureInfo.InvariantCulture, out var value)) { throw new InvalidDataException( diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectStreamsParseTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectStreamsParseTests.cs index 1d5a14bd..6c039508 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectStreamsParseTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectStreamsParseTests.cs @@ -1061,6 +1061,30 @@ public sealed class MTConnectStreamsParseTests observation.DataItemId.ShouldBe("ds"); } + [Fact] + public void The_indentation_between_child_entries_is_not_part_of_the_value() + { + // The parser reads forward-only off an XmlReader, and the value it reports is the + // concatenation of the text it meets on the way. That concatenation only matches what the + // XDocument it replaced reported because whitespace-only text nodes are dropped + // (XmlReaderSettings.IgnoreWhitespace, which is what XDocument.Load defaulted to) — so + // entries separated by a line break must still read "12", never "1\n 2". Nothing else in + // this file can tell the two settings apart: the DATA_SET case above writes its entries + // adjacent, and every other observation is a single text node that Trim() cleans up either + // way. The observation index codes a structured observation BadNotSupported, so the value + // itself is not what matters here — the setting is, and this is the only test that moves + // when it changes. + var result = MTConnectStreamsParser.Parse(StreamsDocument( + """ + + 1 + 2 + + """)); + + result.Observations.ShouldHaveSingleItem().Value.ShouldBe("12"); + } + [Fact] public void A_plain_text_observation_containing_spaces_is_NOT_flagged_structured() {